Re: [libgpiod PATCH] core: Fix line_bulk_foreach_line invalid memory access

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Wed, Feb 2, 2022 at 12:42 PM Joel Stanley <joel@xxxxxxxxx> wrote:
>
> Running libgpiod applications under valgrind results in the following
> warning:
>
> ==3006== Invalid read of size 8
> ==3006==    at 0x10C867: line_request_values (core.c:711)
> ==3006==    by 0x10CDA6: gpiod_line_request_bulk (core.c:849)
> ==3006==    by 0x10AE27: main (gpioset.c:323)
> ==3006==  Address 0x4a4d370 is 0 bytes after a block of size 16 alloc'd
> ==3006==    at 0x483F790: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==3006==    by 0x10B884: gpiod_line_bulk_new (core.c:109)
> ==3006==    by 0x10DBB0: gpiod_chip_get_lines (helpers.c:24)
> ==3006==    by 0x10ADC3: main (gpioset.c:313)
>
> This is because the foreach loop reads the next value before checking
> that index is still in bounds.
>
> Add a test to avoid reading past the end of the allocation.
>
> This bug is not present a released version of libgpiod.
>
> Fixes: 2b02d7ae1aa6 ("treewide: rework struct gpiod_line_bulk")
> Signed-off-by: Joel Stanley <joel@xxxxxxxxx>
> ---
>  lib/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/core.c b/lib/core.c
> index 6ef09baec0f5..4463a7014776 100644
> --- a/lib/core.c
> +++ b/lib/core.c
> @@ -178,7 +178,8 @@ GPIOD_API void gpiod_line_bulk_foreach_line(struct gpiod_line_bulk *bulk,
>  #define line_bulk_foreach_line(bulk, line, index)                      \
>         for ((index) = 0, (line) = (bulk)->lines[0];                    \
>              (index) < (bulk)->num_lines;                               \
> -            (index)++, (line) = (bulk)->lines[(index)])
> +            (index)++,                                                 \
> +            (line) = (index) < (bulk)->num_lines ? (bulk)->lines[(index)] : NULL)
>
>  GPIOD_API bool gpiod_is_gpiochip_device(const char *path)
>  {
> --
> 2.34.1
>

I'll skip this because this entire struct is going away in v2 and the
bug is not present in v1.6.x.

Bart