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@soleen.com>
---
 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);
 		}
-- 
2.43.0

On Fri, Mar 27 2026, Pasha Tatashin 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.

Nit: KHO has absolutely no way to defend against maliciously crafted
data. If the previous kernel is malicious, why would it try to play
around with session strings when it can directly manipulate the
serialization data structures and the memory they point to? There would
be no way to detect or defend against those. I don't think KHO should
even try to defend against malicious data. It should only care about
corrupted data and bugs in the previous kernel.

The only real way to safeguard against malicious kernels is to have some
sort of chain of trust mechanism like kernel signing. That is of course
out of scope for KHO.

So please, if you do a v4, drop the "or maliciously crafted".

The patch itself LGTM.

Reviewed-by: Pratyush Yadav (Google) <pratyush@kernel.org>

>
> 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@soleen.com>
[...]

-- 
Regards,
Pratyush Yadav

On Tue, Mar 31, 2026 at 5:50 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>
> On Fri, Mar 27 2026, Pasha Tatashin 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.
>
> Nit: KHO has absolutely no way to defend against maliciously crafted
> data. If the previous kernel is malicious, why would it try to play
> around with session strings when it can directly manipulate the
> serialization data structures and the memory they point to? There would
> be no way to detect or defend against those. I don't think KHO should
> even try to defend against malicious data. It should only care about
> corrupted data and bugs in the previous kernel.
>
> The only real way to safeguard against malicious kernels is to have some
> sort of chain of trust mechanism like kernel signing. That is of course
> out of scope for KHO.
>
> So please, if you do a v4, drop the "or maliciously crafted".

Makes sense, will do it if there is another version.

>
> The patch itself LGTM.
>
> Reviewed-by: Pratyush Yadav (Google) <pratyush@kernel.org>

Thanks.

Pasha

>
> >
> > 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@soleen.com>
> [...]
>
> --
> Regards,
> Pratyush Yadav

On Thu, Mar 26, 2026 at 11:33 PM Pasha Tatashin
<pasha.tatashin@soleen.com> 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@soleen.com>
> ---
>  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
>

Hi Pasha,

On Fri, Mar 27 2026, Pasha Tatashin wrote:

> On Thu, Mar 26, 2026 at 11:33 PM Pasha Tatashin
> <pasha.tatashin@soleen.com> 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@soleen.com>
>> ---
>>  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.

I think you misunderstood. Sashiko brings up a very good point. The
problem is not that we don't attempt the deserialization again, the
problem is that this code path doesn't set err.

So this results in is_deserialized == true, but err == 0 even though
deserialization failed. So the next attempt to open /dev/liveupdate will
succeed since

	if (is_deserialized)
		return err;

will return 0. So I think you need to do:

	err = PTR_ERR(session);
	return err;

To make sure this error code gets recorded and the next open of
/dev/liveupdate also fails.

Anyway, this isn't directly related to this patch but it is a real bug
that should be fixed in a separate patch.

-- 
Regards,
Pratyush Yadav