Re: [PATCH v3 01/10] liveupdate: Safely print untrusted strings

[Pasha Tatashin]

On Thu, Mar 26, 2026 at 11:33 PM Pasha Tatashin
<pasha.tatashin@xxxxxxxxxx> wrote:
>
> Deserialized strings from KHO data (such as file handler compatible
> strings and session names) are provided by the previous kernel and
> might not be null-terminated if the data is corrupted or maliciously
> crafted.
>
> When printing these strings in error messages, use the %.*s format
> specifier with the maximum buffer size to prevent out-of-bounds reads
> into adjacent kernel memory.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx>
> ---
>  kernel/liveupdate/luo_file.c    | 3 ++-
>  kernel/liveupdate/luo_session.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> index 5acee4174bf0..a6d98fc75d25 100644
> --- a/kernel/liveupdate/luo_file.c
> +++ b/kernel/liveupdate/luo_file.c
> @@ -785,7 +785,8 @@ int luo_file_deserialize(struct luo_file_set *file_set,
>                 }
>
>                 if (!handler_found) {
> -                       pr_warn("No registered handler for compatible '%s'\n",
> +                       pr_warn("No registered handler for compatible '%.*s'\n",
> +                               (int)sizeof(file_ser[i].compatible),
>                                 file_ser[i].compatible);
>                         return -ENOENT;
>                 }
> diff --git a/kernel/liveupdate/luo_session.c b/kernel/liveupdate/luo_session.c
> index 25ae704d7787..8c76dece679b 100644
> --- a/kernel/liveupdate/luo_session.c
> +++ b/kernel/liveupdate/luo_session.c
> @@ -544,7 +544,8 @@ int luo_session_deserialize(void)
>
>                 session = luo_session_alloc(sh->ser[i].name);
>                 if (IS_ERR(session)) {
> -                       pr_warn("Failed to allocate session [%s] during deserialization %pe\n",
> +                       pr_warn("Failed to allocate session [%.*s] during deserialization %pe\n",
> +                               (int)sizeof(sh->ser[i].name),
>                                 sh->ser[i].name, session);
>                         return PTR_ERR(session);
>                 }

Lol, Sashiko went a little overboard and gave this patch two
"Critical" findings:

1. If a registered file handler uses a compatible string equal to or longer than
the buffer, and the untrusted string matches it without a null terminator,
strcmp() could read past the bounds of file_ser[i].compatible.

B.S.: The length of the string is ABI, and fh->compatible is a
NULL-terminated string provided by the current kernel. In the future,
we can replace strcmp() with strncmp(), but it is not a high-priority
issue.

2. By returning PTR_ERR(session) directly without updating the static err
variable, subsequent calls will see is_deserialized as true and return 0.

This is regarding luo_session_deserialize(), that is the intended
behavior. We attempt deserialization exactly once, and if it fails,
some resources stay "leaked" and inaccessible to the user until the
next reboot. This is the safest approach to avoid data leaks.


> --
> 2.43.0
>