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

From: Kent Gibson
Date: Tue Jun 06 2023 - 06:19:15 EST


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;

to also not repeat the zeroing that the kcalloc() did.

Too many options. Let me know which you prefer.

Cheers,
Kent.