Re: [PATCH] gpio-sim: fix memory corruption when adding named lines and unnamed hogs

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Jun 6, 2023 at 12:19 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Tue, Jun 06, 2023 at 12:01:53PM +0200, Bartosz Golaszewski wrote:
> > On Tue, Jun 6, 2023 at 7:13 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > >
> > > When constructing the sim, gpio-sim constructs an array of named lines,
> > > sized based on the largest offset of any named line, and then initializes
> > > that array with the names of all lines, including unnamed hogs with higher
> > > offsets.  In doing so it writes NULLs beyond the extent of the array.
> > >
> > > Add a check that only named lines are used to initialize the array.
> > >
> > > Fixes: cb8c474e79be ("gpio: sim: new testing module")
> > > Signed-off-by: Kent Gibson <warthog618@xxxxxxxxx>
> > > ---
> > >
> > > After writing the comment above, and looking at the code again, it may be
> > > clearer to instead check that the offset is within the bounds of the
> > > array.  Or do both.  Consider that my review.
> > >
> >
> > Like:
> >
> > if (line->offset <= max_offset)
> >     line_names[line->offset] = line->name;
> >
> > ? If so, then I agree it makes the purpose of the check clearer.
> >
>
> Using line_names_size might be even clearer.
>
> So, either that or
>
> if (line->name && (line->offset <= max_offset))
>     line_names[line->offset] = line->name;
>

Yeah, let's go with this one.

Bart

> to also not repeat the zeroing that the kcalloc() did.
>
> Too many options.  Let me know which you prefer.
>
> Cheers,
> Kent.
>
>
>