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

[Bartosz Golaszewski <brgl@xxxxxxxx>]

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.

Bart

> Cheers,
> Kent.
>
>  drivers/gpio/gpio-sim.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
> index e5dfd636c63c..923e8ff53128 100644
> --- a/drivers/gpio/gpio-sim.c
> +++ b/drivers/gpio/gpio-sim.c
> @@ -721,8 +721,10 @@ static char **gpio_sim_make_line_names(struct gpio_sim_bank *bank,
>         if (!line_names)
>                 return ERR_PTR(-ENOMEM);
>
> -       list_for_each_entry(line, &bank->line_list, siblings)
> -               line_names[line->offset] = line->name;
> +       list_for_each_entry(line, &bank->line_list, siblings) {
> +               if (line->name)
> +                       line_names[line->offset] = line->name;
> +       }
>
>         return line_names;
>  }
> --
> 2.40.1
>