[PATCH v2] platform/x86: hp-bioscfg: fix slab-out-of-bounds write in hp_convert_hexstr_to_str

[PATCH v2] platform/x86: hp-bioscfg: fix slab-out-of-bounds write in hp_convert_hexstr_to_str

[Muhammad Bilal]

hp_convert_hexstr_to_str() decodes an ACPI string made up of
space-separated ASCII hex-byte tokens of the form "0xHH". For
example, the four-byte input "0x41" decodes to the single
character 'A', and the nine-byte input "0x09 0x41" decodes to
"\tA": each token is read in a five-byte step (four characters for
the token, one for the trailing delimiter), and produces one output
byte, or two if that byte is '\\', '\r', '\n', or '\t', which are
written back out as a backslash followed by the matching letter.

The output buffer is sized with kmalloc(input_len, GFP_KERNEL), the
raw encoded length of the input, not the decoded length. For
well-formed input this is generous, since five input bytes never
decode to more than two output bytes. But input_len comes directly
from the ACPI string length reported by firmware and isn't
guaranteed to respect the five-byte encoding, so a short input_len
can undersize the allocation. With input_len == 1, only one byte is
allocated, yet decoding still produces at least one output byte
plus the NUL terminator written unconditionally afterwards, so two
bytes are needed. That terminator write then lands one byte past
the end of the allocation.

KASAN caught exactly this during BIOS attribute enumeration on
boot, triggered by a one-byte encoded input value:

BUG: KASAN: slab-out-of-bounds in hp_convert_hexstr_to_str+0x6d8/0x710 [hp_bioscfg]
Write of size 1 at addr ffff8881032e5d81 by task (udev-worker)/520
The buggy address is located 0 bytes to the right of
allocated 1-byte region [ffff8881032e5d80, ffff8881032e5d81)

Size the allocation to the worst-case decoded length instead of the
raw input length: two output bytes for every five-byte input chunk
(DIV_ROUND_UP(input_len, 5)), plus one byte for the terminator.

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
v2: Expand the commit message to spell out the hex-token input
    format and decoded output with a worked example, and explain
    exactly how a short input_len undersizes the allocation, per
    Ilpo Järvinen's review. No code change from v1.

 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 22c198680903..42331cf90581 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -442,7 +442,7 @@ int hp_convert_hexstr_to_str(const char *input, u32 input_len, char **str, int *
 	*len = 0;
 	*str = NULL;
 
-	new_str = kmalloc(input_len, GFP_KERNEL);
+	new_str = kmalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
 	if (!new_str)
 		return -ENOMEM;
 
-- 
2.55.0

Re: [PATCH v2] platform/x86: hp-bioscfg: fix slab-out-of-bounds write in hp_convert_hexstr_to_str

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3555 bytes --]

On Tue, 15 Sep 2026, Muhammad Bilal wrote:

> hp_convert_hexstr_to_str() decodes an ACPI string made up of
> space-separated ASCII hex-byte tokens of the form "0xHH". For
> example, the four-byte input "0x41" decodes to the single
> character 'A', and the nine-byte input "0x09 0x41" decodes to
> "\tA": each token is read in a five-byte step (four characters for
> the token, one for the trailing delimiter), and produces one output
> byte, or two if that byte is '\\', '\r', '\n', or '\t', which are
> written back out as a backslash followed by the matching letter.
> 
> The output buffer is sized with kmalloc(input_len, GFP_KERNEL), the
> raw encoded length of the input, not the decoded length. For
> well-formed input this is generous, since five input bytes never
> decode to more than two output bytes. But input_len comes directly
> from the ACPI string length reported by firmware and isn't
> guaranteed to respect the five-byte encoding, so a short input_len
> can undersize the allocation. With input_len == 1, only one byte is
> allocated, yet decoding still produces at least one output byte
> plus the NUL terminator written unconditionally afterwards, so two
> bytes are needed. That terminator write then lands one byte past
> the end of the allocation.

Thanks, very clear now.

I've applied this to the review-ilpo-next branch now, but please see 
below for one additional thing.

> KASAN caught exactly this during BIOS attribute enumeration on
> boot, triggered by a one-byte encoded input value:
> 
> BUG: KASAN: slab-out-of-bounds in hp_convert_hexstr_to_str+0x6d8/0x710 [hp_bioscfg]
> Write of size 1 at addr ffff8881032e5d81 by task (udev-worker)/520
> The buggy address is located 0 bytes to the right of
> allocated 1-byte region [ffff8881032e5d80, ffff8881032e5d81)
> 
> Size the allocation to the worst-case decoded length instead of the
> raw input length: two output bytes for every five-byte input chunk
> (DIV_ROUND_UP(input_len, 5)), plus one byte for the terminator.
> 
> Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> v2: Expand the commit message to spell out the hex-token input
>     format and decoded output with a worked example, and explain
>     exactly how a short input_len undersizes the allocation, per
>     Ilpo Järvinen's review. No code change from v1.
> 
>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> index 22c198680903..42331cf90581 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> @@ -442,7 +442,7 @@ int hp_convert_hexstr_to_str(const char *input, u32 input_len, char **str, int *
>  	*len = 0;
>  	*str = NULL;
>  
> -	new_str = kmalloc(input_len, GFP_KERNEL);
> +	new_str = kmalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
>  	if (!new_str)
>  		return -ENOMEM;
>  
> 

I think the buffer should be allocated with kzalloc() to ensure there's no 
potential leakage when/if the output buffer is filled only partially 
which seems well possible given your description. So it would warrant 
another patch on top of this change (as I've applied this now).


And your other patches are not forgotten, I'll get to them while 
processing the rest of the patch queue.


-- 
 i.

[PATCH] platform/x86: hp-bioscfg: zero the hex-string decode buffer in hp_convert_hexstr_to_str

[Muhammad Bilal]

hp_convert_hexstr_to_str() allocates its output buffer for the
worst-case decoded length, then fills in only as many bytes as the
input actually decodes to before shrinking the allocation down to
that length with krealloc(). Well-formed input can decode to
noticeably fewer bytes than the worst case, so the buffer is
frequently only partially written by the time it is realloc'd and
returned to the caller.

Use kzalloc() instead of kmalloc() for the initial allocation, so
any unused capacity starts out zeroed instead of holding leftover
heap contents, rather than relying on every current and future
caller and code path to fill the buffer exactly.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Applies on top of "platform/x86: hp-bioscfg: fix slab-out-of-bounds
write in hp_convert_hexstr_to_str" (the DIV_ROUND_UP sizing fix),
which Ilpo has applied to review-ilpo-next but is not yet in
mainline. Sent as its own patch rather than a v3 of that one, since
the sizing fix itself was applied as-is; this is the separate change
requested on top of it.
---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index ff28db7..2dab9c0 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -442,7 +442,7 @@ int hp_convert_hexstr_to_str(const char *input, u32 input_len, char **str, int *
 	*len = 0;
 	*str = NULL;
 
-	new_str = kmalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
+	new_str = kzalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
 	if (!new_str)
 		return -ENOMEM;
 
-- 
2.43.0

Re: [PATCH] platform/x86: hp-bioscfg: zero the hex-string decode buffer in hp_convert_hexstr_to_str

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]

On Wed, 16 Sep 2026, Muhammad Bilal wrote:

> hp_convert_hexstr_to_str() allocates its output buffer for the
> worst-case decoded length, then fills in only as many bytes as the
> input actually decodes to before shrinking the allocation down to
> that length with krealloc(). Well-formed input can decode to
> noticeably fewer bytes than the worst case, so the buffer is
> frequently only partially written by the time it is realloc'd and
> returned to the caller.
> 
> Use kzalloc() instead of kmalloc() for the initial allocation, so
> any unused capacity starts out zeroed instead of holding leftover
> heap contents, rather than relying on every current and future
> caller and code path to fill the buffer exactly.
> 
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> Applies on top of "platform/x86: hp-bioscfg: fix slab-out-of-bounds
> write in hp_convert_hexstr_to_str" (the DIV_ROUND_UP sizing fix),
> which Ilpo has applied to review-ilpo-next but is not yet in
> mainline. Sent as its own patch rather than a v3 of that one, since
> the sizing fix itself was applied as-is; this is the separate change
> requested on top of it.

Thanks, applied to review-ilpo-next.

In future, please try to add parenthesis into function names in the 
shortlog (on Subject line) as well so I don't need to manually add them 
myself.

--
 i.

> ---
>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> index ff28db7..2dab9c0 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> @@ -442,7 +442,7 @@ int hp_convert_hexstr_to_str(const char *input, u32 input_len, char **str, int *
>  	*len = 0;
>  	*str = NULL;
>  
> -	new_str = kmalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
> +	new_str = kzalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
>  	if (!new_str)
>  		return -ENOMEM;
>  
> 

Re: [PATCH] platform/x86: hp-bioscfg: zero the hex-string decode buffer in hp_convert_hexstr_to_str

[Muhammad Bilal]

Thanks, Ilpo.

Noted. I'll include the parentheses in function names in the Subject
line for future patches.

Thanks for the review and for applying the patch.

--
Muhammad


On Wed, Sep 16, 2026 at 3:29 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 16 Sep 2026, Muhammad Bilal wrote:
>
> > hp_convert_hexstr_to_str() allocates its output buffer for the
> > worst-case decoded length, then fills in only as many bytes as the
> > input actually decodes to before shrinking the allocation down to
> > that length with krealloc(). Well-formed input can decode to
> > noticeably fewer bytes than the worst case, so the buffer is
> > frequently only partially written by the time it is realloc'd and
> > returned to the caller.
> >
> > Use kzalloc() instead of kmalloc() for the initial allocation, so
> > any unused capacity starts out zeroed instead of holding leftover
> > heap contents, rather than relying on every current and future
> > caller and code path to fill the buffer exactly.
> >
> > Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> > ---
> > Applies on top of "platform/x86: hp-bioscfg: fix slab-out-of-bounds
> > write in hp_convert_hexstr_to_str" (the DIV_ROUND_UP sizing fix),
> > which Ilpo has applied to review-ilpo-next but is not yet in
> > mainline. Sent as its own patch rather than a v3 of that one, since
> > the sizing fix itself was applied as-is; this is the separate change
> > requested on top of it.
>
> Thanks, applied to review-ilpo-next.
>
> In future, please try to add parenthesis into function names in the
> shortlog (on Subject line) as well so I don't need to manually add them
> myself.
>
> --
>  i.
>
> > ---
> >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > index ff28db7..2dab9c0 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > @@ -442,7 +442,7 @@ int hp_convert_hexstr_to_str(const char *input, u32 input_len, char **str, int *
> >       *len = 0;
> >       *str = NULL;
> >
> > -     new_str = kmalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
> > +     new_str = kzalloc(2 * DIV_ROUND_UP(input_len, 5) + 1, GFP_KERNEL);
> >       if (!new_str)
> >               return -ENOMEM;
> >
> >