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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- 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.
>