[PATCH v6] platform/x86: hp-bioscfg: fix heap OOB in hp_enforce_single_line_input()

[PATCH v6] platform/x86: hp-bioscfg: fix heap OOB in hp_enforce_single_line_input()

[Muhammad Bilal]

The input is copied with kstrdup(), which stops at the first NUL, but
the full write size is passed to hp_enforce_single_line_input(). With
an embedded NUL the helper reads, and can write one byte, past the
copy. Return -EINVAL if the input contains a NUL.
 
Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.8): writing "A\0"
followed by 4093 bytes of "B" to current_password 100 times returns
-EINVAL every time, without reading into the heap.
 
Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v6:
 - Return -EINVAL on embedded NUL instead of limiting the scan (Ilpo)
 
v5: https://lore.kernel.org/all/20261007134711.473857-1-meatuni001@gmail.com/
 
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 876fc35d035b..e306bd8b1e72 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -162,6 +162,10 @@ int hp_enforce_single_line_input(char *buf, size_t count)
 {
 	char *p;
 
+	/* buf is a string, so a NUL before count is invalid input */
+	if (strnlen(buf, count) < count)
+		return -EINVAL;
+
 	p = memchr(buf, '\n', count);
 
 	if (p == buf + count - 1)

base-commit: 8667bd30fd033295b3038ce374c551d1059538ba
-- 
2.56.0

Re: [PATCH v6] platform/x86: hp-bioscfg: fix heap OOB in hp_enforce_single_line_input()

[Ilpo Järvinen]

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

On Fri, 16 Oct 2026, Muhammad Bilal wrote:

> The input is copied with kstrdup(), which stops at the first NUL, but
> the full write size is passed to hp_enforce_single_line_input(). With
> an embedded NUL the helper reads, and can write one byte, past the
> copy. Return -EINVAL if the input contains a NUL.
>  
> Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.8): writing "A\0"
> followed by 4093 bytes of "B" to current_password 100 times returns
> -EINVAL every time, without reading into the heap.
>  
> Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> Cc: stable@vger.kernel.org
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> Changes in v6:
>  - Return -EINVAL on embedded NUL instead of limiting the scan (Ilpo)
>  
> v5: https://lore.kernel.org/all/20261007134711.473857-1-meatuni001@gmail.com/
>  
>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> index 876fc35d035b..e306bd8b1e72 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> @@ -162,6 +162,10 @@ int hp_enforce_single_line_input(char *buf, size_t count)
>  {
>  	char *p;
>  
> +	/* buf is a string, so a NUL before count is invalid input */
> +	if (strnlen(buf, count) < count)

Why use n variant?

> +		return -EINVAL;
> +
>  	p = memchr(buf, '\n', count);
>  
>  	if (p == buf + count - 1)
> 
> base-commit: 8667bd30fd033295b3038ce374c551d1059538ba
> 

-- 
 i.

Re: [PATCH v6] platform/x86: hp-bioscfg: fix heap OOB in hp_enforce_single_line_input()

[Muhammad Bilal]

Hi Ilpo,

> Why use n variant?
strnlen() keeps the read within count, which is the size of buf as far
as this helper knows, so it does not depend on the caller NUL
terminating the buffer. Both current callers pass a kstrdup() copy, so
strlen() works too. I can switch to it in v7 if you prefer.

Thanks
Muhammad

On Wed, Oct 21, 2026 at 2:19 AM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Fri, 16 Oct 2026, Muhammad Bilal wrote:
>
> > The input is copied with kstrdup(), which stops at the first NUL, but
> > the full write size is passed to hp_enforce_single_line_input(). With
> > an embedded NUL the helper reads, and can write one byte, past the
> > copy. Return -EINVAL if the input contains a NUL.
> >
> > Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.8): writing "A\0"
> > followed by 4093 bytes of "B" to current_password 100 times returns
> > -EINVAL every time, without reading into the heap.
> >
> > Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> > Cc: stable@vger.kernel.org
> > Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> > ---
> > Changes in v6:
> >  - Return -EINVAL on embedded NUL instead of limiting the scan (Ilpo)
> >
> > v5: https://lore.kernel.org/all/20261007134711.473857-1-meatuni001@gmail.com/
> >
> >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > index 876fc35d035b..e306bd8b1e72 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > @@ -162,6 +162,10 @@ int hp_enforce_single_line_input(char *buf, size_t count)
> >  {
> >       char *p;
> >
> > +     /* buf is a string, so a NUL before count is invalid input */
> > +     if (strnlen(buf, count) < count)
>
> Why use n variant?
>
> > +             return -EINVAL;
> > +
> >       p = memchr(buf, '\n', count);
> >
> >       if (p == buf + count - 1)
> >
> > base-commit: 8667bd30fd033295b3038ce374c551d1059538ba
> >
>
> --
>  i.

Re: [PATCH v6] platform/x86: hp-bioscfg: fix heap OOB in hp_enforce_single_line_input()

[Ilpo Järvinen]

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

On Wed, 21 Oct 2026, Muhammad Bilal wrote:

> Hi Ilpo,
> 
> > Why use n variant?
> strnlen() keeps the read within count, which is the size of buf as far
> as this helper knows, so it does not depend on the caller NUL
> terminating the buffer. Both current callers pass a kstrdup() copy, so
> strlen() works too. I can switch to it in v7 if you prefer.

As the contract here is the input must be a string, I don't see any need 
to limit the length with n variant so I do prefer strlen(). Also, we know 
that it should not be longer than count anyway.

--
 i.

> Thanks
> Muhammad
> 
> On Wed, Oct 21, 2026 at 2:19 AM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Fri, 16 Oct 2026, Muhammad Bilal wrote:
> >
> > > The input is copied with kstrdup(), which stops at the first NUL, but
> > > the full write size is passed to hp_enforce_single_line_input(). With
> > > an embedded NUL the helper reads, and can write one byte, past the
> > > copy. Return -EINVAL if the input contains a NUL.
> > >
> > > Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.8): writing "A\0"
> > > followed by 4093 bytes of "B" to current_password 100 times returns
> > > -EINVAL every time, without reading into the heap.
> > >
> > > Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> > > Cc: stable@vger.kernel.org
> > > Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> > > ---
> > > Changes in v6:
> > >  - Return -EINVAL on embedded NUL instead of limiting the scan (Ilpo)
> > >
> > > v5: https://lore.kernel.org/all/20261007134711.473857-1-meatuni001@gmail.com/
> > >
> > >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > > index 876fc35d035b..e306bd8b1e72 100644
> > > --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > > +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > > @@ -162,6 +162,10 @@ int hp_enforce_single_line_input(char *buf, size_t count)
> > >  {
> > >       char *p;
> > >
> > > +     /* buf is a string, so a NUL before count is invalid input */
> > > +     if (strnlen(buf, count) < count)
> >
> > Why use n variant?
> >
> > > +             return -EINVAL;
> > > +
> > >       p = memchr(buf, '\n', count);
> > >
> > >       if (p == buf + count - 1)
> > >
> > > base-commit: 8667bd30fd033295b3038ce374c551d1059538ba
> > >
> >
> > --
> >  i.
> 

Re: [PATCH v6] platform/x86: hp-bioscfg: fix heap OOB in hp_enforce_single_line_input()

[Ilpo Järvinen]

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

On Wed, 21 Oct 2026, Ilpo Järvinen wrote:

> On Wed, 21 Oct 2026, Muhammad Bilal wrote:
> 
> > Hi Ilpo,
> > 
> > > Why use n variant?
> > strnlen() keeps the read within count, which is the size of buf as far
> > as this helper knows, so it does not depend on the caller NUL
> > terminating the buffer. Both current callers pass a kstrdup() copy, so
> > strlen() works too. I can switch to it in v7 if you prefer.
> 
> As the contract here is the input must be a string, I don't see any need 
> to limit the length with n variant so I do prefer strlen(). Also, we know 
> that it should not be longer than count anyway.

I only now notice Andy's comments which seem better suggestion in general.

-- 
 i.

Re: [PATCH v6] platform/x86: hp-bioscfg: fix heap OOB in hp_enforce_single_line_input()

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 07:53:52AM +0500, Muhammad Bilal wrote:
> The input is copied with kstrdup(), which stops at the first NUL, but
> the full write size is passed to hp_enforce_single_line_input(). With
> an embedded NUL the helper reads, and can write one byte, past the
> copy. Return -EINVAL if the input contains a NUL.
>  
> Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.8): writing "A\0"
> followed by 4093 bytes of "B" to current_password 100 times returns
> -EINVAL every time, without reading into the heap.

...

>  {
>  	char *p;
>  
> +	/* buf is a string, so a NUL before count is invalid input */
> +	if (strnlen(buf, count) < count)
> +		return -EINVAL;
> +
>  	p = memchr(buf, '\n', count);
>  
>  	if (p == buf + count - 1)

Let's look at the full implementation

int hp_enforce_single_line_input(char *buf, size_t count)
{
	char *p;

	p = memchr(buf, '\n', count);

	if (p == buf + count - 1)
		*p = '\0'; /* strip trailing newline */
	else if (p)
		return -EINVAL;  /* enforce single line input */

	return 0;
}

With your enforcement in place the rest can be simplified and refactored.
The hint is to use strnchrnul().

int hp_enforce_single_line_input(char *buf, size_t count)
{
	char *p;

	p = strnchrnul(buf, count, '\n');

	if (p == buf + count - 1)
		*p = '\0'; /* strip trailing newline */
	else if (p < buf + count - 1)
		return -EINVAL;  /* enforce single line input */

	return 0;
}

(the above done with the less intrusive changes)

-- 
With Best Regards,
Andy Shevchenko