lkml.org 
[lkml]   [2020]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v12 4/4] gpio: xilinx: Utilize generic bitmap_get_value and _set_value
On Sun, Oct 18, 2020 at 11:44 PM Syed Nayyar Waris <syednwaris@gmail.com> wrote:
>
> This patch reimplements the xgpio_set_multiple() function in
> drivers/gpio/gpio-xilinx.c to use the new generic functions:
> bitmap_get_value() and bitmap_set_value(). The code is now simpler
> to read and understand. Moreover, instead of looping for each bit
> in xgpio_set_multiple() function, now we can check each channel at
> a time and save cycles.

This now causes -Wtype-limits warnings in linux-next with gcc-10:

> + u32 *const state = chip->gpio_state;
> + unsigned int *const width = chip->gpio_width;
> +
> + DECLARE_BITMAP(old, 64);
> + DECLARE_BITMAP(new, 64);
> + DECLARE_BITMAP(changed, 64);
> +
> + spin_lock_irqsave(&chip->gpio_lock[0], flags);
> + spin_lock(&chip->gpio_lock[1]);
> +
> + bitmap_set_value(old, state[0], 0, width[0]);
> + bitmap_set_value(old, state[1], width[0], width[1]);

In file included from ../include/linux/cpumask.h:12,
from ../arch/x86/include/asm/cpumask.h:5,
from ../arch/x86/include/asm/msr.h:11,
from ../arch/x86/include/asm/processor.h:22,
from ../arch/x86/include/asm/timex.h:5,
from ../include/linux/timex.h:65,
from ../include/linux/time32.h:13,
from ../include/linux/time.h:73,
from ../include/linux/stat.h:19,
from ../include/linux/module.h:13,
from ../drivers/gpio/gpio-xilinx.c:11:
../include/linux/bitmap.h:639:18: warning: array subscript [1,
67108864] is outside array bounds of 'long unsigned int[1]'
[-Warray-bounds]
639 | map[index + 1] |= value >> space;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
In file included from ../include/linux/kasan-checks.h:5,
from ../include/asm-generic/rwonce.h:26,
from ./arch/x86/include/generated/asm/rwonce.h:1,
from ../include/linux/compiler.h:246,
from ../include/linux/build_bug.h:5,
from ../include/linux/bits.h:22,
from ../include/linux/bitops.h:6,
from ../drivers/gpio/gpio-xilinx.c:8:
../drivers/gpio/gpio-xilinx.c:144:17: note: while referencing 'old'
144 | DECLARE_BITMAP(old, 64);
| ^~~
../include/linux/types.h:11:16: note: in definition of macro 'DECLARE_BITMAP'
11 | unsigned long name[BITS_TO_LONGS(bits)]
| ^~~~
In file included from ../include/linux/cpumask.h:12,
from ../arch/x86/include/asm/cpumask.h:5,
from ../arch/x86/include/asm/msr.h:11,
from ../arch/x86/include/asm/processor.h:22,
from ../arch/x86/include/asm/timex.h:5,
from ../include/linux/timex.h:65,
from ../include/linux/time32.h:13,
from ../include/linux/time.h:73,
from ../include/linux/stat.h:19,
from ../include/linux/module.h:13,
from ../drivers/gpio/gpio-xilinx.c:11:

The compiler clearly tries to do range-checking here and notices
that the index into the fixed-length array on the stack is not correctly
bounded. It seems this would happen whenever width[0] + width[1]
is larger than 64.

I have just submitted patches for all other -Wtype-limits warnings
and would like to enable this option by default. Can you try to find
a way to make this code safer? I would expect that you need a
variant of bitmap_set_value() that takes an explicit ceiling here,
and checks the stand and nbits values against that.

Arnd

\
 
 \ /
  Last update: 2020-10-29 23:45    [W:1.331 / U:1.384 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site