* [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]'
@ 2023-10-17 0:14 kernel test robot
2023-11-06 16:31 ` Alexander Lobakin
0 siblings, 1 reply; 15+ messages in thread
From: kernel test robot @ 2023-10-17 0:14 UTC (permalink / raw)
To: Alexander Lobakin; +Cc: oe-kbuild-all
tree: https://github.com/alobakin/linux pfcp
head: 9183a3eb639912169a3d3e2be4f25556b465919b
commit: c8a652cdcc0964510f108726b3da0784d1bc0cd2 [11/19] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}()
config: x86_64-randconfig-004-20231017 (https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/
All warnings (new ones prefixed by >>):
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:23,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:60,
from arch/x86/include/asm/preempt.h:9,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:7,
from include/linux/slab.h:16,
from include/linux/resource_ext.h:11,
from include/linux/acpi.h:13,
from drivers/gpio/gpio-pca953x.c:11:
drivers/gpio/gpio-pca953x.c: In function 'pca953x_probe':
>> include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds]
642 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);
| ^~
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:21,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/gpio/gpio-pca953x.c:11:
drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val'
1032 | DECLARE_BITMAP(val, MAX_LINE);
| ^~~
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:23,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:60,
from arch/x86/include/asm/preempt.h:9,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:7,
from include/linux/slab.h:16,
from include/linux/resource_ext.h:11,
from include/linux/acpi.h:13,
from drivers/gpio/gpio-pca953x.c:11:
>> include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds]
642 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);
| ^~
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:21,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/gpio/gpio-pca953x.c:11:
drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val'
1032 | DECLARE_BITMAP(val, MAX_LINE);
| ^~~
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:23,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:60,
from arch/x86/include/asm/preempt.h:9,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:7,
from include/linux/slab.h:16,
from include/linux/resource_ext.h:11,
from include/linux/acpi.h:13,
from drivers/gpio/gpio-pca953x.c:11:
include/linux/bitmap.h:643:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds]
643 | 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:21,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/gpio/gpio-pca953x.c:11:
drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val'
1032 | DECLARE_BITMAP(val, MAX_LINE);
| ^~~
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:23,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:60,
from arch/x86/include/asm/preempt.h:9,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:7,
from include/linux/slab.h:16,
from include/linux/resource_ext.h:11,
from include/linux/acpi.h:13,
from drivers/gpio/gpio-pca953x.c:11:
include/linux/bitmap.h:643:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds]
643 | 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:21,
from include/linux/ioport.h:13,
from include/linux/acpi.h:12,
from drivers/gpio/gpio-pca953x.c:11:
drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val'
1032 | DECLARE_BITMAP(val, MAX_LINE);
| ^~~
include/linux/types.h:11:16: note: in definition of macro 'DECLARE_BITMAP'
11 | unsigned long name[BITS_TO_LONGS(bits)]
| ^~~~
vim +642 include/linux/bitmap.h
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 603
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 604 /**
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 605 * bitmap_write - write n-bit value within a memory region
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 606 * @map: address to the bitmap memory region
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 607 * @value: value to write, clamped to nbits
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 608 * @start: bit offset of the n-bit value
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 609 * @nbits: size of value in bits, nonzero, up to BITS_PER_LONG.
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 610 *
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 611 * bitmap_write() behaves as-if implemented as @nbits calls of __assign_bit(),
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 612 * i.e. bits beyond @nbits are ignored:
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 613 *
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 614 * for (bit = 0; bit < nbits; bit++)
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 615 * __assign_bit(start + bit, bitmap, val & BIT(bit));
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 616 */
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 617 static inline void bitmap_write(unsigned long *map,
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 618 unsigned long value,
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 619 unsigned long start, unsigned long nbits)
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 620 {
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 621 size_t index;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 622 unsigned long offset;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 623 unsigned long space;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 624 unsigned long mask;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 625 bool fit;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 626
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 627 if (unlikely(!nbits))
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 628 return;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 629
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 630 mask = BITMAP_LAST_WORD_MASK(nbits);
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 631 value &= mask;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 632 offset = start % BITS_PER_LONG;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 633 space = BITS_PER_LONG - offset;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 634 fit = space >= nbits;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 635 index = BIT_WORD(start);
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 636
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 637 map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start));
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 638 map[index] |= value << offset;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 639 if (fit)
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 640 return;
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 641
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 @642 map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 643 map[index + 1] |= (value >> space);
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 644 }
c8ac468adba7fe Syed Nayyar Waris 2023-10-11 645
:::::: The code at line 642 was first introduced by commit
:::::: c8ac468adba7fe930ec22f392d5f127b768deccc lib/bitmap: add bitmap_{read,write}()
:::::: TO: Syed Nayyar Waris <syednwaris@gmail.com>
:::::: CC: Alexander Lobakin <aleksander.lobakin@intel.com>
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-10-17 0:14 [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' kernel test robot @ 2023-11-06 16:31 ` Alexander Lobakin 2023-11-06 18:23 ` Andy Shevchenko 2023-11-07 13:22 ` Yury Norov 0 siblings, 2 replies; 15+ messages in thread From: Alexander Lobakin @ 2023-11-06 16:31 UTC (permalink / raw) To: Alexander Potapenko Cc: Andy Shevchenko, Yury Norov, Syed Nayyar Waris, kernel test robot, oe-kbuild-all, linux-kernel@vger.kernel.org BTW, I have this in my inbox: From: Kernel Test Robot <lkp@intel.com> Date: Tue, 17 Oct 2023 08:14:51 +0800 > tree: https://github.com/alobakin/linux pfcp > head: 9183a3eb639912169a3d3e2be4f25556b465919b > commit: c8a652cdcc0964510f108726b3da0784d1bc0cd2 [11/19] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}() So it happened after I converted bitmap_{get,set}_value8() so that they use bitmap_{read,write}(). > config: x86_64-randconfig-004-20231017 (https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/config) > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ > > All warnings (new ones prefixed by >>): > > 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:23, > from arch/x86/include/asm/cpufeature.h:5, > from arch/x86/include/asm/thread_info.h:53, > from include/linux/thread_info.h:60, > from arch/x86/include/asm/preempt.h:9, > from include/linux/preempt.h:79, > from include/linux/spinlock.h:56, > from include/linux/mmzone.h:8, > from include/linux/gfp.h:7, > from include/linux/slab.h:16, > from include/linux/resource_ext.h:11, > from include/linux/acpi.h:13, > from drivers/gpio/gpio-pca953x.c:11: > drivers/gpio/gpio-pca953x.c: In function 'pca953x_probe': >>> include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds] > 642 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > | ^~ > 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:21, > from include/linux/ioport.h:13, > from include/linux/acpi.h:12, > from drivers/gpio/gpio-pca953x.c:11: > drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val' > 1032 | DECLARE_BITMAP(val, MAX_LINE); > | ^~~ > 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:23, > from arch/x86/include/asm/cpufeature.h:5, > from arch/x86/include/asm/thread_info.h:53, > from include/linux/thread_info.h:60, > from arch/x86/include/asm/preempt.h:9, > from include/linux/preempt.h:79, > from include/linux/spinlock.h:56, > from include/linux/mmzone.h:8, > from include/linux/gfp.h:7, > from include/linux/slab.h:16, > from include/linux/resource_ext.h:11, > from include/linux/acpi.h:13, > from drivers/gpio/gpio-pca953x.c:11: >>> include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds] > 642 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > | ^~ > 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:21, > from include/linux/ioport.h:13, > from include/linux/acpi.h:12, > from drivers/gpio/gpio-pca953x.c:11: > drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val' > 1032 | DECLARE_BITMAP(val, MAX_LINE); > | ^~~ > 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:23, > from arch/x86/include/asm/cpufeature.h:5, > from arch/x86/include/asm/thread_info.h:53, > from include/linux/thread_info.h:60, > from arch/x86/include/asm/preempt.h:9, > from include/linux/preempt.h:79, > from include/linux/spinlock.h:56, > from include/linux/mmzone.h:8, > from include/linux/gfp.h:7, > from include/linux/slab.h:16, > from include/linux/resource_ext.h:11, > from include/linux/acpi.h:13, > from drivers/gpio/gpio-pca953x.c:11: > include/linux/bitmap.h:643:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds] > 643 | 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:21, > from include/linux/ioport.h:13, > from include/linux/acpi.h:12, > from drivers/gpio/gpio-pca953x.c:11: > drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val' > 1032 | DECLARE_BITMAP(val, MAX_LINE); > | ^~~ > 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:23, > from arch/x86/include/asm/cpufeature.h:5, > from arch/x86/include/asm/thread_info.h:53, > from include/linux/thread_info.h:60, > from arch/x86/include/asm/preempt.h:9, > from include/linux/preempt.h:79, > from include/linux/spinlock.h:56, > from include/linux/mmzone.h:8, > from include/linux/gfp.h:7, > from include/linux/slab.h:16, > from include/linux/resource_ext.h:11, > from include/linux/acpi.h:13, > from drivers/gpio/gpio-pca953x.c:11: > include/linux/bitmap.h:643:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' [-Warray-bounds] > 643 | 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:21, > from include/linux/ioport.h:13, > from include/linux/acpi.h:12, > from drivers/gpio/gpio-pca953x.c:11: > drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val' > 1032 | DECLARE_BITMAP(val, MAX_LINE); > | ^~~ > include/linux/types.h:11:16: note: in definition of macro 'DECLARE_BITMAP' > 11 | unsigned long name[BITS_TO_LONGS(bits)] > | ^~~~ > > > vim +642 include/linux/bitmap.h > > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 603 > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 604 /** > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 605 * bitmap_write - write n-bit value within a memory region > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 606 * @map: address to the bitmap memory region > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 607 * @value: value to write, clamped to nbits > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 608 * @start: bit offset of the n-bit value > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 609 * @nbits: size of value in bits, nonzero, up to BITS_PER_LONG. > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 610 * > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 611 * bitmap_write() behaves as-if implemented as @nbits calls of __assign_bit(), > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 612 * i.e. bits beyond @nbits are ignored: > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 613 * > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 614 * for (bit = 0; bit < nbits; bit++) > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 615 * __assign_bit(start + bit, bitmap, val & BIT(bit)); > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 616 */ > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 617 static inline void bitmap_write(unsigned long *map, > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 618 unsigned long value, > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 619 unsigned long start, unsigned long nbits) > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 620 { > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 621 size_t index; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 622 unsigned long offset; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 623 unsigned long space; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 624 unsigned long mask; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 625 bool fit; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 626 > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 627 if (unlikely(!nbits)) > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 628 return; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 629 > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 630 mask = BITMAP_LAST_WORD_MASK(nbits); > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 631 value &= mask; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 632 offset = start % BITS_PER_LONG; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 633 space = BITS_PER_LONG - offset; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 634 fit = space >= nbits; For that particular case, @offset is not a compile-time constant, thus the compiler can't assume anything. @space and @fit are as well due to that. > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 635 index = BIT_WORD(start); > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 636 > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 637 map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start)); > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 638 map[index] |= value << offset; > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 639 if (fit) > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 640 return; ...which means the compiler doesn't know whether this condition will return from the function or not. > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 641 > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 @642 map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 643 map[index + 1] |= (value >> space); However, it knows that the bitmap has only 1 long and indexes above 0 are not valid. Not sure how to approach this :z It was also captured on the version you sent 2 weeks ago, so this could've been resolved already. > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 644 } > c8ac468adba7fe Syed Nayyar Waris 2023-10-11 645 > > :::::: The code at line 642 was first introduced by commit > :::::: c8ac468adba7fe930ec22f392d5f127b768deccc lib/bitmap: add bitmap_{read,write}() > > :::::: TO: Syed Nayyar Waris <syednwaris@gmail.com> > :::::: CC: Alexander Lobakin <aleksander.lobakin@intel.com> > Thanks, Olek ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-06 16:31 ` Alexander Lobakin @ 2023-11-06 18:23 ` Andy Shevchenko 2023-11-07 13:21 ` Alexander Lobakin 2023-11-07 13:22 ` Yury Norov 1 sibling, 1 reply; 15+ messages in thread From: Andy Shevchenko @ 2023-11-06 18:23 UTC (permalink / raw) To: Alexander Lobakin Cc: Alexander Potapenko, Yury Norov, Syed Nayyar Waris, kernel test robot, oe-kbuild-all, linux-kernel@vger.kernel.org On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote: > > | Reported-by: kernel test robot <lkp@intel.com> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ > Not sure how to approach this :z It was also captured on the version you > sent 2 weeks ago, so this could've been resolved already. Is it in the repository already? if so, we should revert it. Otherwise you have time to think and fix. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-06 18:23 ` Andy Shevchenko @ 2023-11-07 13:21 ` Alexander Lobakin 2023-11-07 16:33 ` Alexander Potapenko 0 siblings, 1 reply; 15+ messages in thread From: Alexander Lobakin @ 2023-11-07 13:21 UTC (permalink / raw) To: Andy Shevchenko Cc: Alexander Potapenko, Yury Norov, Syed Nayyar Waris, kernel test robot, oe-kbuild-all, linux-kernel@vger.kernel.org From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Date: Mon, 6 Nov 2023 20:23:52 +0200 > On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote: > >>> | Reported-by: kernel test robot <lkp@intel.com> >>> | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ > >> Not sure how to approach this :z It was also captured on the version you >> sent 2 weeks ago, so this could've been resolved already. > > Is it in the repository already? if so, we should revert it. > Otherwise you have time to think and fix. Nah, neither Alex' series nor mine. And I'd say this should rather be resolved in the functions Alex introduce. Thanks, Olek ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 13:21 ` Alexander Lobakin @ 2023-11-07 16:33 ` Alexander Potapenko 2023-11-07 16:44 ` Alexander Lobakin 0 siblings, 1 reply; 15+ messages in thread From: Alexander Potapenko @ 2023-11-07 16:33 UTC (permalink / raw) To: Alexander Lobakin Cc: Andy Shevchenko, Yury Norov, Syed Nayyar Waris, kernel test robot, oe-kbuild-all, linux-kernel@vger.kernel.org On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin <aleksander.lobakin@intel.com> wrote: > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Date: Mon, 6 Nov 2023 20:23:52 +0200 > > > On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote: > > > >>> | Reported-by: kernel test robot <lkp@intel.com> > >>> | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ > > > >> Not sure how to approach this :z It was also captured on the version you > >> sent 2 weeks ago, so this could've been resolved already. > > > > Is it in the repository already? if so, we should revert it. > > Otherwise you have time to think and fix. > > Nah, neither Alex' series nor mine. And I'd say this should rather be > resolved in the functions Alex introduce. > > Thanks, > Olek Sorry, I couldn't reproduce the problem using the instructions at https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce locally, maybe that's because I only have gcc-11 and higher. But if I'm understanding correctly what's going on, then GCC will be reporting the same issue in the following code: ======================================================= #include <stddef.h> #include <stdbool.h> #define BITS_PER_LONG 64 #define unlikely(x) x #define UL(x) (x##UL) #define GENMASK(h, l) \ (((~UL(0)) - (UL(1) << (l)) + 1) & \ (~UL(0) >> (BITS_PER_LONG - 1 - (h)))) #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) inline void bitmap_write(unsigned long *map, unsigned long value, unsigned long start, unsigned long nbits) { size_t index; unsigned long offset; unsigned long space; unsigned long mask; bool fit; if (unlikely(!nbits)) return; mask = BITMAP_LAST_WORD_MASK(nbits); value &= mask; offset = start % BITS_PER_LONG; space = BITS_PER_LONG - offset; fit = space >= nbits; index = BIT_WORD(start); map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start)); map[index] |= value << offset; if (fit) return; map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); map[index + 1] |= (value >> space); } unsigned long foo(unsigned int n) { unsigned long bm[1] = {0}; bitmap_write(bm, 1, n, 2); return bm[0]; } ======================================================= (see also https://godbolt.org/z/GfGfYje53) If so, the problem is not specific to GCC 9, trunk GCC also barks on this code: ======================================================= In function 'bitmap_write', inlined from 'bitmap_write' at <source>:15:13, inlined from 'foo' at <source>:47:7: <source>:40:12: warning: array subscript 1 is outside array bounds of 'long unsigned int[1]' [-Warray-bounds=] 40 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); | ~~~^~~~~~~~~~~ ======================================================= If this is true for the code in drivers/gpio/gpio-pca953x.c, suppressing the report for GCC 9 won't help for other versions. Given that this report is isolated in a single file, we probably need to give the compiler some hints about the range of values passed to bitmap_write() rather than suppressing the optimizations. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 16:33 ` Alexander Potapenko @ 2023-11-07 16:44 ` Alexander Lobakin 2023-11-07 17:24 ` Alexander Lobakin 2023-11-07 23:25 ` Kees Cook 0 siblings, 2 replies; 15+ messages in thread From: Alexander Lobakin @ 2023-11-07 16:44 UTC (permalink / raw) To: Alexander Potapenko Cc: Andy Shevchenko, Yury Norov, Syed Nayyar Waris, Kees Cook, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org From: Alexander Potapenko <glider@google.com> Date: Tue, 7 Nov 2023 17:33:56 +0100 > On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin > <aleksander.lobakin@intel.com> wrote: >> >> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Date: Mon, 6 Nov 2023 20:23:52 +0200 >> >>> On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote: >>> >>>>> | Reported-by: kernel test robot <lkp@intel.com> >>>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ >>> >>>> Not sure how to approach this :z It was also captured on the version you >>>> sent 2 weeks ago, so this could've been resolved already. >>> >>> Is it in the repository already? if so, we should revert it. >>> Otherwise you have time to think and fix. >> >> Nah, neither Alex' series nor mine. And I'd say this should rather be >> resolved in the functions Alex introduce. >> >> Thanks, >> Olek > > Sorry, I couldn't reproduce the problem using the instructions at > https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce > locally, maybe that's because I only have gcc-11 and higher. > > But if I'm understanding correctly what's going on, then GCC will be > reporting the same issue in the following code: > > ======================================================= > #include <stddef.h> > #include <stdbool.h> > > #define BITS_PER_LONG 64 > #define unlikely(x) x > #define UL(x) (x##UL) > #define GENMASK(h, l) \ > (((~UL(0)) - (UL(1) << (l)) + 1) & \ > (~UL(0) >> (BITS_PER_LONG - 1 - (h)))) > > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) > #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) > > inline void bitmap_write(unsigned long *map, > unsigned long value, > unsigned long start, unsigned long nbits) > { > size_t index; > unsigned long offset; > unsigned long space; > unsigned long mask; > bool fit; > > if (unlikely(!nbits)) > return; > > mask = BITMAP_LAST_WORD_MASK(nbits); > value &= mask; > offset = start % BITS_PER_LONG; > space = BITS_PER_LONG - offset; > fit = space >= nbits; > index = BIT_WORD(start); > > map[index] &= (fit ? (~(mask << offset)) : > ~BITMAP_FIRST_WORD_MASK(start)); > map[index] |= value << offset; > if (fit) > return; > > map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > map[index + 1] |= (value >> space); > } > > unsigned long foo(unsigned int n) { > unsigned long bm[1] = {0}; > bitmap_write(bm, 1, n, 2); > return bm[0]; > } > ======================================================= > (see also https://godbolt.org/z/GfGfYje53) > > If so, the problem is not specific to GCC 9, trunk GCC also barks on this code: > > ======================================================= > In function 'bitmap_write', > inlined from 'bitmap_write' at <source>:15:13, > inlined from 'foo' at <source>:47:7: > <source>:40:12: warning: array subscript 1 is outside array bounds of > 'long unsigned int[1]' [-Warray-bounds=] > 40 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > | ~~~^~~~~~~~~~~ > ======================================================= > > If this is true for the code in drivers/gpio/gpio-pca953x.c, > suppressing the report for GCC 9 won't help for other versions. > Given that this report is isolated in a single file, we probably need I tested it on GCC 9 using modified make.cross from lkp and it triggers on one more file: drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': ./include/linux/bitmap.h:601:18: error: array subscript [1, 288230376151711744] is outside array bounds of 'long unsigned int[1]' [-Werror=array-bounds] > to give the compiler some hints about the range of values passed to > bitmap_write() rather than suppressing the optimizations. OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it correctly, rather shuts up the compiler in cases like this one. I've been thinking of using __member_size() from fortify-string.h, we could probably optimize the object code even a bit more while silencing this warning. Adding Kees, maybe he'd like to participate in sorting this out as well. Thanks, Olek ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 16:44 ` Alexander Lobakin @ 2023-11-07 17:24 ` Alexander Lobakin 2023-11-07 18:32 ` Yury Norov 2023-11-07 23:25 ` Kees Cook 1 sibling, 1 reply; 15+ messages in thread From: Alexander Lobakin @ 2023-11-07 17:24 UTC (permalink / raw) To: Alexander Potapenko Cc: Andy Shevchenko, Yury Norov, Syed Nayyar Waris, Kees Cook, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org From: Alexander Lobakin <aleksander.lobakin@intel.com> Date: Tue, 7 Nov 2023 17:44:00 +0100 > From: Alexander Potapenko <glider@google.com> > Date: Tue, 7 Nov 2023 17:33:56 +0100 > >> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin >> <aleksander.lobakin@intel.com> wrote: [...] > I tested it on GCC 9 using modified make.cross from lkp and it triggers > on one more file: > > drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': > ./include/linux/bitmap.h:601:18: error: array subscript [1, > 288230376151711744] is outside array bounds of 'long unsigned int[1]' > [-Werror=array-bounds] > >> to give the compiler some hints about the range of values passed to >> bitmap_write() rather than suppressing the optimizations. > > OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it > correctly, rather shuts up the compiler in cases like this one. > > I've been thinking of using __member_size() from fortify-string.h, we > could probably optimize the object code even a bit more while silencing > this warning. > Adding Kees, maybe he'd like to participate in sorting this out as well. This one seems to work. At least previously mad GCC 9.3.0 now sits quietly, as if I added OPTIMIZER_HIDE_VAR() as Yury suggested. Note that ideally @map should be marked as `POS` in both cases to help Clang, but `POS` gets undefined at the end of fortify-string.h, so I decided to not do that within this draft. Thanks, Olek --- diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index e8031a157db5..efa0a0287d7c 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -589,12 +589,14 @@ static inline unsigned long bitmap_read(const unsigned long *map, size_t index = BIT_WORD(start); unsigned long offset = start % BITS_PER_LONG; unsigned long space = BITS_PER_LONG - offset; + const size_t map_size = __member_size(map); unsigned long value_low, value_high; if (unlikely(!nbits || nbits > BITS_PER_LONG)) return 0; - if (space >= nbits) + if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX && + index + 1 >= map_size / sizeof(long)) || space >= nbits) return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits); value_low = map[index] & BITMAP_FIRST_WORD_MASK(start); @@ -620,6 +622,7 @@ static inline unsigned long bitmap_read(const unsigned long *map, static inline void bitmap_write(unsigned long *map, unsigned long value, unsigned long start, unsigned long nbits) { + const size_t map_size = __member_size(map); size_t index; unsigned long offset; unsigned long space; @@ -638,7 +641,9 @@ static inline void bitmap_write(unsigned long *map, unsigned long value, map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start)); map[index] |= value << offset; - if (fit) + + if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX && + index + 1 >= map_size / sizeof(long)) || fit) return; map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 17:24 ` Alexander Lobakin @ 2023-11-07 18:32 ` Yury Norov 2023-11-07 18:52 ` Alexander Lobakin 0 siblings, 1 reply; 15+ messages in thread From: Yury Norov @ 2023-11-07 18:32 UTC (permalink / raw) To: Alexander Lobakin Cc: Alexander Potapenko, Andy Shevchenko, Syed Nayyar Waris, Kees Cook, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org On Tue, Nov 07, 2023 at 06:24:04PM +0100, Alexander Lobakin wrote: > From: Alexander Lobakin <aleksander.lobakin@intel.com> > Date: Tue, 7 Nov 2023 17:44:00 +0100 > > > From: Alexander Potapenko <glider@google.com> > > Date: Tue, 7 Nov 2023 17:33:56 +0100 > > > >> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin > >> <aleksander.lobakin@intel.com> wrote: > > [...] > > > I tested it on GCC 9 using modified make.cross from lkp and it triggers > > on one more file: > > > > drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': > > ./include/linux/bitmap.h:601:18: error: array subscript [1, > > 288230376151711744] is outside array bounds of 'long unsigned int[1]' > > [-Werror=array-bounds] > > > >> to give the compiler some hints about the range of values passed to > >> bitmap_write() rather than suppressing the optimizations. > > > > OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it > > correctly, rather shuts up the compiler in cases like this one. > > > > I've been thinking of using __member_size() from fortify-string.h, we > > could probably optimize the object code even a bit more while silencing > > this warning. > > Adding Kees, maybe he'd like to participate in sorting this out as well. > > This one seems to work. At least previously mad GCC 9.3.0 now sits > quietly, as if I added OPTIMIZER_HIDE_VAR() as Yury suggested. What's wrong with OPTIMIZER_HIDE_VAR()? The problem is clearly on GCC side, namely - it doesn't realize that the map[index+1] fetch is conditional. And moreover, it's fixed in later stable builds. I tested 12 and 13, and both are silent. > Note that ideally @map should be marked as `POS` in both cases to help > Clang, but `POS` gets undefined at the end of fortify-string.h, so I > decided to not do that within this draft. > > Thanks, > Olek > --- > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h > index e8031a157db5..efa0a0287d7c 100644 > --- a/include/linux/bitmap.h > +++ b/include/linux/bitmap.h > @@ -589,12 +589,14 @@ static inline unsigned long bitmap_read(const > unsigned long *map, > size_t index = BIT_WORD(start); > unsigned long offset = start % BITS_PER_LONG; > unsigned long space = BITS_PER_LONG - offset; > + const size_t map_size = __member_size(map); > unsigned long value_low, value_high; > > if (unlikely(!nbits || nbits > BITS_PER_LONG)) > return 0; > > - if (space >= nbits) > + if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX && > + index + 1 >= map_size / sizeof(long)) || space >= nbits) > return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits); This silences the compiler, but breaks the code logic and hides potential bugs. After the fix, the following code will become legit: DECLARE_BITMAP(bitmap, 64); bitmap_fill(bitmap, 64) char ret = bitmap_read(bitmap, 60, 8); // OK, return 0b00001111 Before this change, the return value would be undef: 0xXXXX1111, and it would (should) trigger Warray-bounds on compile time, because it's a compile-time boundary violation. On runtime KASAN, UBSAN and whatever *SAN would most likely be silenced too with your fix. So no, this one doesn't seem to work. > value_low = map[index] & BITMAP_FIRST_WORD_MASK(start); > @@ -620,6 +622,7 @@ static inline unsigned long bitmap_read(const > unsigned long *map, > static inline void bitmap_write(unsigned long *map, unsigned long value, > unsigned long start, unsigned long nbits) > { > + const size_t map_size = __member_size(map); > size_t index; > unsigned long offset; > unsigned long space; > @@ -638,7 +641,9 @@ static inline void bitmap_write(unsigned long *map, > unsigned long value, > > map[index] &= (fit ? (~(mask << offset)) : > ~BITMAP_FIRST_WORD_MASK(start)); > map[index] |= value << offset; > - if (fit) > + > + if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX && > + index + 1 >= map_size / sizeof(long)) || fit) > return; > > map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 18:32 ` Yury Norov @ 2023-11-07 18:52 ` Alexander Lobakin 2023-11-07 19:24 ` Yury Norov 0 siblings, 1 reply; 15+ messages in thread From: Alexander Lobakin @ 2023-11-07 18:52 UTC (permalink / raw) To: Yury Norov Cc: Alexander Potapenko, Andy Shevchenko, Syed Nayyar Waris, Kees Cook, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org From: Yury Norov <yury.norov@gmail.com> Date: Tue, 7 Nov 2023 10:32:06 -0800 > On Tue, Nov 07, 2023 at 06:24:04PM +0100, Alexander Lobakin wrote: >> From: Alexander Lobakin <aleksander.lobakin@intel.com> >> Date: Tue, 7 Nov 2023 17:44:00 +0100 >> >>> From: Alexander Potapenko <glider@google.com> >>> Date: Tue, 7 Nov 2023 17:33:56 +0100 >>> >>>> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin >>>> <aleksander.lobakin@intel.com> wrote: >> >> [...] >> >>> I tested it on GCC 9 using modified make.cross from lkp and it triggers >>> on one more file: >>> >>> drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': >>> ./include/linux/bitmap.h:601:18: error: array subscript [1, >>> 288230376151711744] is outside array bounds of 'long unsigned int[1]' >>> [-Werror=array-bounds] >>> >>>> to give the compiler some hints about the range of values passed to >>>> bitmap_write() rather than suppressing the optimizations. >>> >>> OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it >>> correctly, rather shuts up the compiler in cases like this one. >>> >>> I've been thinking of using __member_size() from fortify-string.h, we >>> could probably optimize the object code even a bit more while silencing >>> this warning. >>> Adding Kees, maybe he'd like to participate in sorting this out as well. >> >> This one seems to work. At least previously mad GCC 9.3.0 now sits >> quietly, as if I added OPTIMIZER_HIDE_VAR() as Yury suggested. > > What's wrong with OPTIMIZER_HIDE_VAR()? The problem is clearly on GCC > side, namely - it doesn't realize that the map[index+1] fetch is > conditional. It's totally fine for me to use it, this one is just an alternative (well, a bit broken as per below). > > And moreover, it's fixed in later stable builds. I tested 12 and 13, > and both are silent. Yeah, this happens only on GCC 9 on my side. I thought on older GCCs `-Warray-bounds` is not enabled due to false-positives. > >> Note that ideally @map should be marked as `POS` in both cases to help >> Clang, but `POS` gets undefined at the end of fortify-string.h, so I >> decided to not do that within this draft. >> >> Thanks, >> Olek >> --- >> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h >> index e8031a157db5..efa0a0287d7c 100644 >> --- a/include/linux/bitmap.h >> +++ b/include/linux/bitmap.h >> @@ -589,12 +589,14 @@ static inline unsigned long bitmap_read(const >> unsigned long *map, >> size_t index = BIT_WORD(start); >> unsigned long offset = start % BITS_PER_LONG; >> unsigned long space = BITS_PER_LONG - offset; >> + const size_t map_size = __member_size(map); >> unsigned long value_low, value_high; >> >> if (unlikely(!nbits || nbits > BITS_PER_LONG)) >> return 0; >> >> - if (space >= nbits) >> + if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX && >> + index + 1 >= map_size / sizeof(long)) || space >= nbits) >> return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits); > > This silences the compiler, but breaks the code logic and hides potential bugs. > After the fix, the following code will become legit: > > DECLARE_BITMAP(bitmap, 64); > > bitmap_fill(bitmap, 64) > char ret = bitmap_read(bitmap, 60, 8); // OK, return 0b00001111 > > Before this change, the return value would be undef: 0xXXXX1111, and > it would (should) trigger Warray-bounds on compile time, because it's > a compile-time boundary violation. Oh you're right, I didn't think about this. Your approach seems optimal unless hardening folks have anything else. I don't see bitmap_{read,write}() mini-series applied anywhere in your tree, maybe Alex could incorporate your patch into it and resubmit? > > On runtime KASAN, UBSAN and whatever *SAN would most likely be silenced > too with your fix. So no, this one doesn't seem to work. > >> value_low = map[index] & BITMAP_FIRST_WORD_MASK(start); >> @@ -620,6 +622,7 @@ static inline unsigned long bitmap_read(const >> unsigned long *map, >> static inline void bitmap_write(unsigned long *map, unsigned long value, >> unsigned long start, unsigned long nbits) >> { >> + const size_t map_size = __member_size(map); >> size_t index; >> unsigned long offset; >> unsigned long space; >> @@ -638,7 +641,9 @@ static inline void bitmap_write(unsigned long *map, >> unsigned long value, >> >> map[index] &= (fit ? (~(mask << offset)) : >> ~BITMAP_FIRST_WORD_MASK(start)); >> map[index] |= value << offset; >> - if (fit) >> + >> + if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX && >> + index + 1 >= map_size / sizeof(long)) || fit) >> return; >> >> map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); Thanks, Olek ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 18:52 ` Alexander Lobakin @ 2023-11-07 19:24 ` Yury Norov 2023-11-08 10:07 ` Alexander Potapenko 2023-11-08 12:28 ` Alexander Lobakin 0 siblings, 2 replies; 15+ messages in thread From: Yury Norov @ 2023-11-07 19:24 UTC (permalink / raw) To: Alexander Lobakin Cc: Alexander Potapenko, Andy Shevchenko, Syed Nayyar Waris, Kees Cook, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org On Tue, Nov 07, 2023 at 07:52:19PM +0100, Alexander Lobakin wrote: > From: Yury Norov <yury.norov@gmail.com> > Date: Tue, 7 Nov 2023 10:32:06 -0800 > > > On Tue, Nov 07, 2023 at 06:24:04PM +0100, Alexander Lobakin wrote: > >> From: Alexander Lobakin <aleksander.lobakin@intel.com> > >> Date: Tue, 7 Nov 2023 17:44:00 +0100 > >> > >>> From: Alexander Potapenko <glider@google.com> > >>> Date: Tue, 7 Nov 2023 17:33:56 +0100 > >>> > >>>> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin > >>>> <aleksander.lobakin@intel.com> wrote: > >> > >> [...] > >> > >>> I tested it on GCC 9 using modified make.cross from lkp and it triggers > >>> on one more file: > >>> > >>> drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': > >>> ./include/linux/bitmap.h:601:18: error: array subscript [1, > >>> 288230376151711744] is outside array bounds of 'long unsigned int[1]' > >>> [-Werror=array-bounds] > >>> > >>>> to give the compiler some hints about the range of values passed to > >>>> bitmap_write() rather than suppressing the optimizations. > >>> > >>> OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it > >>> correctly, rather shuts up the compiler in cases like this one. > >>> > >>> I've been thinking of using __member_size() from fortify-string.h, we > >>> could probably optimize the object code even a bit more while silencing > >>> this warning. > >>> Adding Kees, maybe he'd like to participate in sorting this out as well. > >> > >> This one seems to work. At least previously mad GCC 9.3.0 now sits > >> quietly, as if I added OPTIMIZER_HIDE_VAR() as Yury suggested. > > > > What's wrong with OPTIMIZER_HIDE_VAR()? The problem is clearly on GCC > > side, namely - it doesn't realize that the map[index+1] fetch is > > conditional. > > It's totally fine for me to use it, this one is just an alternative > (well, a bit broken as per below). OK, guys, that's even worse. The 12 and 13 don't fire the warning because Warray-bounds is explicitly disabled for gcc-11+. Check 0da6e5fd6c372 ("gcc: disable '-Warray-bounds' for gcc-13 too"). I'll test how gcc-10 builds it, and if it's broken too, it's worth to shift the threshold in init/Kconfig. Let me check it later today. [...] > Oh you're right, I didn't think about this. Your approach seems optimal > unless hardening folks have anything else. > > I don't see bitmap_{read,write}() mini-series applied anywhere in your I'll not take the code unless there are real kernel users for it. Your compressor is still under development AFAIK, so I'm going to pull bitmap_read/write with ip_tunnel series, if it comes first. > tree, maybe Alex could incorporate your patch into it and resubmit? Yes, that's what I asked him to do. But let's put it on hold while I'm testing different compilers. Thanks, Yury ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 19:24 ` Yury Norov @ 2023-11-08 10:07 ` Alexander Potapenko 2023-11-08 12:28 ` Alexander Lobakin 1 sibling, 0 replies; 15+ messages in thread From: Alexander Potapenko @ 2023-11-08 10:07 UTC (permalink / raw) To: Yury Norov Cc: Alexander Lobakin, Andy Shevchenko, Syed Nayyar Waris, Kees Cook, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org On Tue, Nov 7, 2023 at 8:24 PM Yury Norov <yury.norov@gmail.com> wrote: > > On Tue, Nov 07, 2023 at 07:52:19PM +0100, Alexander Lobakin wrote: > > From: Yury Norov <yury.norov@gmail.com> > > Date: Tue, 7 Nov 2023 10:32:06 -0800 > > > > > On Tue, Nov 07, 2023 at 06:24:04PM +0100, Alexander Lobakin wrote: > > >> From: Alexander Lobakin <aleksander.lobakin@intel.com> > > >> Date: Tue, 7 Nov 2023 17:44:00 +0100 > > >> > > >>> From: Alexander Potapenko <glider@google.com> > > >>> Date: Tue, 7 Nov 2023 17:33:56 +0100 > > >>> > > >>>> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin > > >>>> <aleksander.lobakin@intel.com> wrote: > > >> > > >> [...] > > >> > > >>> I tested it on GCC 9 using modified make.cross from lkp and it triggers > > >>> on one more file: > > >>> > > >>> drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': > > >>> ./include/linux/bitmap.h:601:18: error: array subscript [1, > > >>> 288230376151711744] is outside array bounds of 'long unsigned int[1]' > > >>> [-Werror=array-bounds] > > >>> > > >>>> to give the compiler some hints about the range of values passed to > > >>>> bitmap_write() rather than suppressing the optimizations. > > >>> > > >>> OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it > > >>> correctly, rather shuts up the compiler in cases like this one. > > >>> > > >>> I've been thinking of using __member_size() from fortify-string.h, we > > >>> could probably optimize the object code even a bit more while silencing > > >>> this warning. > > >>> Adding Kees, maybe he'd like to participate in sorting this out as well. > > >> > > >> This one seems to work. At least previously mad GCC 9.3.0 now sits > > >> quietly, as if I added OPTIMIZER_HIDE_VAR() as Yury suggested. > > > > > > What's wrong with OPTIMIZER_HIDE_VAR()? The problem is clearly on GCC > > > side, namely - it doesn't realize that the map[index+1] fetch is > > > conditional. > > > > It's totally fine for me to use it, this one is just an alternative > > (well, a bit broken as per below). > > OK, guys, that's even worse. The 12 and 13 don't fire the warning > because Warray-bounds is explicitly disabled for gcc-11+. Check > 0da6e5fd6c372 ("gcc: disable '-Warray-bounds' for gcc-13 too"). I'll > test how gcc-10 builds it, and if it's broken too, it's worth to shift > the threshold in init/Kconfig. Yes, that's my point. According to Godbolt, GCC versions 9.1 to 13.2 (trunk included) are reporting the same false positive on the code I posted above. In 5a41237ad1d4b62008f93163af1d9b1da90729d8 ("gcc: disable -Warray-bounds for gcc-11 too") Linus says that "Older gcc versions end up being increasingly less relevant", so I think there won't be objections against extending this rule to GCC 9 and 10. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 19:24 ` Yury Norov 2023-11-08 10:07 ` Alexander Potapenko @ 2023-11-08 12:28 ` Alexander Lobakin 1 sibling, 0 replies; 15+ messages in thread From: Alexander Lobakin @ 2023-11-08 12:28 UTC (permalink / raw) To: Yury Norov Cc: Alexander Potapenko, Andy Shevchenko, Syed Nayyar Waris, Kees Cook, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org From: Yury Norov <yury.norov@gmail.com> Date: Tue, 7 Nov 2023 11:24:49 -0800 > On Tue, Nov 07, 2023 at 07:52:19PM +0100, Alexander Lobakin wrote: >> From: Yury Norov <yury.norov@gmail.com> >> Date: Tue, 7 Nov 2023 10:32:06 -0800 >> >>> On Tue, Nov 07, 2023 at 06:24:04PM +0100, Alexander Lobakin wrote: >>>> From: Alexander Lobakin <aleksander.lobakin@intel.com> >>>> Date: Tue, 7 Nov 2023 17:44:00 +0100 >>>> >>>>> From: Alexander Potapenko <glider@google.com> >>>>> Date: Tue, 7 Nov 2023 17:33:56 +0100 >>>>> >>>>>> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin >>>>>> <aleksander.lobakin@intel.com> wrote: >>>> >>>> [...] >>>> >>>>> I tested it on GCC 9 using modified make.cross from lkp and it triggers >>>>> on one more file: >>>>> >>>>> drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': >>>>> ./include/linux/bitmap.h:601:18: error: array subscript [1, >>>>> 288230376151711744] is outside array bounds of 'long unsigned int[1]' >>>>> [-Werror=array-bounds] >>>>> >>>>>> to give the compiler some hints about the range of values passed to >>>>>> bitmap_write() rather than suppressing the optimizations. >>>>> >>>>> OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it >>>>> correctly, rather shuts up the compiler in cases like this one. >>>>> >>>>> I've been thinking of using __member_size() from fortify-string.h, we >>>>> could probably optimize the object code even a bit more while silencing >>>>> this warning. >>>>> Adding Kees, maybe he'd like to participate in sorting this out as well. >>>> >>>> This one seems to work. At least previously mad GCC 9.3.0 now sits >>>> quietly, as if I added OPTIMIZER_HIDE_VAR() as Yury suggested. >>> >>> What's wrong with OPTIMIZER_HIDE_VAR()? The problem is clearly on GCC >>> side, namely - it doesn't realize that the map[index+1] fetch is >>> conditional. >> >> It's totally fine for me to use it, this one is just an alternative >> (well, a bit broken as per below). > > OK, guys, that's even worse. The 12 and 13 don't fire the warning > because Warray-bounds is explicitly disabled for gcc-11+. Check > 0da6e5fd6c372 ("gcc: disable '-Warray-bounds' for gcc-13 too"). I'll > test how gcc-10 builds it, and if it's broken too, it's worth to shift > the threshold in init/Kconfig. OPTIMIZER_HIDE_VAR() silences GCC 11 on my side (`-Warray-bounds` is enabled via KCFLAGS), but now I'm wondering if it could possibly hide really incorrect cases like you mentioned before (reading 8 bits starting at 60 from a 64-bit bitmap). > > Let me check it later today. > > [...] > >> Oh you're right, I didn't think about this. Your approach seems optimal >> unless hardening folks have anything else. >> >> I don't see bitmap_{read,write}() mini-series applied anywhere in your > > I'll not take the code unless there are real kernel users for it. Your > compressor is still under development AFAIK, so I'm going to pull > bitmap_read/write with ip_tunnel series, if it comes first. > >> tree, maybe Alex could incorporate your patch into it and resubmit? > > Yes, that's what I asked him to do. But let's put it on hold while I'm > testing different compilers. So now I feel like it's a matter of extending the Kconfig threshold like you said. The code is clearly valid. Alternatively, we could trigger a build bug manually when offset, width and map size are compile-time constants and suppress it otherwise. But sounds pretty hacky. > > Thanks, > Yury Thanks, Olek ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 16:44 ` Alexander Lobakin 2023-11-07 17:24 ` Alexander Lobakin @ 2023-11-07 23:25 ` Kees Cook 2023-11-08 0:48 ` Yury Norov 1 sibling, 1 reply; 15+ messages in thread From: Kees Cook @ 2023-11-07 23:25 UTC (permalink / raw) To: Alexander Lobakin Cc: Alexander Potapenko, Andy Shevchenko, Yury Norov, Syed Nayyar Waris, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org On Tue, Nov 07, 2023 at 05:44:00PM +0100, Alexander Lobakin wrote: > From: Alexander Potapenko <glider@google.com> > Date: Tue, 7 Nov 2023 17:33:56 +0100 > > > On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin > > <aleksander.lobakin@intel.com> wrote: > >> > >> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > >> Date: Mon, 6 Nov 2023 20:23:52 +0200 > >> > >>> On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote: > >>> > >>>>> | Reported-by: kernel test robot <lkp@intel.com> > >>>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ > >>> > >>>> Not sure how to approach this :z It was also captured on the version you > >>>> sent 2 weeks ago, so this could've been resolved already. > >>> > >>> Is it in the repository already? if so, we should revert it. > >>> Otherwise you have time to think and fix. > >> > >> Nah, neither Alex' series nor mine. And I'd say this should rather be > >> resolved in the functions Alex introduce. > >> > >> Thanks, > >> Olek > > > > Sorry, I couldn't reproduce the problem using the instructions at > > https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce > > locally, maybe that's because I only have gcc-11 and higher. > > > > But if I'm understanding correctly what's going on, then GCC will be > > reporting the same issue in the following code: > > > > ======================================================= > > #include <stddef.h> > > #include <stdbool.h> > > > > #define BITS_PER_LONG 64 > > #define unlikely(x) x > > #define UL(x) (x##UL) > > #define GENMASK(h, l) \ > > (((~UL(0)) - (UL(1) << (l)) + 1) & \ > > (~UL(0) >> (BITS_PER_LONG - 1 - (h)))) > > > > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > > #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) > > #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) > > > > inline void bitmap_write(unsigned long *map, > > unsigned long value, > > unsigned long start, unsigned long nbits) > > { > > size_t index; > > unsigned long offset; > > unsigned long space; > > unsigned long mask; > > bool fit; > > > > if (unlikely(!nbits)) > > return; > > > > mask = BITMAP_LAST_WORD_MASK(nbits); > > value &= mask; > > offset = start % BITS_PER_LONG; > > space = BITS_PER_LONG - offset; > > fit = space >= nbits; > > index = BIT_WORD(start); > > > > map[index] &= (fit ? (~(mask << offset)) : > > ~BITMAP_FIRST_WORD_MASK(start)); > > map[index] |= value << offset; > > if (fit) > > return; > > > > map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > > map[index + 1] |= (value >> space); > > } > > > > unsigned long foo(unsigned int n) { > > unsigned long bm[1] = {0}; > > bitmap_write(bm, 1, n, 2); > > return bm[0]; > > } > > ======================================================= > > (see also https://godbolt.org/z/GfGfYje53) > > > > If so, the problem is not specific to GCC 9, trunk GCC also barks on this code: > > > > ======================================================= > > In function 'bitmap_write', > > inlined from 'bitmap_write' at <source>:15:13, > > inlined from 'foo' at <source>:47:7: > > <source>:40:12: warning: array subscript 1 is outside array bounds of > > 'long unsigned int[1]' [-Warray-bounds=] > > 40 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > > | ~~~^~~~~~~~~~~ > > ======================================================= > > > > If this is true for the code in drivers/gpio/gpio-pca953x.c, > > suppressing the report for GCC 9 won't help for other versions. > > Given that this report is isolated in a single file, we probably need > > I tested it on GCC 9 using modified make.cross from lkp and it triggers > on one more file: > > drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': > ./include/linux/bitmap.h:601:18: error: array subscript [1, > 288230376151711744] is outside array bounds of 'long unsigned int[1]' > [-Werror=array-bounds] > > > to give the compiler some hints about the range of values passed to > > bitmap_write() rather than suppressing the optimizations. > > OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it > correctly, rather shuts up the compiler in cases like this one. > > I've been thinking of using __member_size() from fortify-string.h, we > could probably optimize the object code even a bit more while silencing > this warning. > Adding Kees, maybe he'd like to participate in sorting this out as well. I'm trying to find all the pieces for this code, so I might be working from the wrong version or something, but I think this is the change: https://github.com/alobakin/linux/commit/66808fb20fed014a522b868322d54daef14a6bd8 and the induced warning is correctly analyzed in this thread (i.e. GCC can't convince itself that it'll never reach the out of bounds access). Does this work? - if (fit) + if (fit || index + 1 >= __member_size(map)) return; -- Kees Cook ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-07 23:25 ` Kees Cook @ 2023-11-08 0:48 ` Yury Norov 0 siblings, 0 replies; 15+ messages in thread From: Yury Norov @ 2023-11-08 0:48 UTC (permalink / raw) To: Kees Cook Cc: Alexander Lobakin, Alexander Potapenko, Andy Shevchenko, Syed Nayyar Waris, kernel test robot, oe-kbuild-all, linux-hardening, linux-kernel@vger.kernel.org On Tue, Nov 07, 2023 at 03:25:01PM -0800, Kees Cook wrote: > On Tue, Nov 07, 2023 at 05:44:00PM +0100, Alexander Lobakin wrote: > > From: Alexander Potapenko <glider@google.com> > > Date: Tue, 7 Nov 2023 17:33:56 +0100 > > > > > On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin > > > <aleksander.lobakin@intel.com> wrote: > > >> > > >> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > >> Date: Mon, 6 Nov 2023 20:23:52 +0200 > > >> > > >>> On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote: > > >>> > > >>>>> | Reported-by: kernel test robot <lkp@intel.com> > > >>>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ > > >>> > > >>>> Not sure how to approach this :z It was also captured on the version you > > >>>> sent 2 weeks ago, so this could've been resolved already. > > >>> > > >>> Is it in the repository already? if so, we should revert it. > > >>> Otherwise you have time to think and fix. > > >> > > >> Nah, neither Alex' series nor mine. And I'd say this should rather be > > >> resolved in the functions Alex introduce. > > >> > > >> Thanks, > > >> Olek > > > > > > Sorry, I couldn't reproduce the problem using the instructions at > > > https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce > > > locally, maybe that's because I only have gcc-11 and higher. > > > > > > But if I'm understanding correctly what's going on, then GCC will be > > > reporting the same issue in the following code: > > > > > > ======================================================= > > > #include <stddef.h> > > > #include <stdbool.h> > > > > > > #define BITS_PER_LONG 64 > > > #define unlikely(x) x > > > #define UL(x) (x##UL) > > > #define GENMASK(h, l) \ > > > (((~UL(0)) - (UL(1) << (l)) + 1) & \ > > > (~UL(0) >> (BITS_PER_LONG - 1 - (h)))) > > > > > > #define BIT_WORD(nr) ((nr) / BITS_PER_LONG) > > > #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) > > > #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) > > > > > > inline void bitmap_write(unsigned long *map, > > > unsigned long value, > > > unsigned long start, unsigned long nbits) > > > { > > > size_t index; > > > unsigned long offset; > > > unsigned long space; > > > unsigned long mask; > > > bool fit; > > > > > > if (unlikely(!nbits)) > > > return; > > > > > > mask = BITMAP_LAST_WORD_MASK(nbits); > > > value &= mask; > > > offset = start % BITS_PER_LONG; > > > space = BITS_PER_LONG - offset; > > > fit = space >= nbits; > > > index = BIT_WORD(start); > > > > > > map[index] &= (fit ? (~(mask << offset)) : > > > ~BITMAP_FIRST_WORD_MASK(start)); > > > map[index] |= value << offset; > > > if (fit) > > > return; > > > > > > map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > > > map[index + 1] |= (value >> space); > > > } > > > > > > unsigned long foo(unsigned int n) { > > > unsigned long bm[1] = {0}; > > > bitmap_write(bm, 1, n, 2); > > > return bm[0]; > > > } > > > ======================================================= > > > (see also https://godbolt.org/z/GfGfYje53) > > > > > > If so, the problem is not specific to GCC 9, trunk GCC also barks on this code: > > > > > > ======================================================= > > > In function 'bitmap_write', > > > inlined from 'bitmap_write' at <source>:15:13, > > > inlined from 'foo' at <source>:47:7: > > > <source>:40:12: warning: array subscript 1 is outside array bounds of > > > 'long unsigned int[1]' [-Warray-bounds=] > > > 40 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits); > > > | ~~~^~~~~~~~~~~ > > > ======================================================= > > > > > > If this is true for the code in drivers/gpio/gpio-pca953x.c, > > > suppressing the report for GCC 9 won't help for other versions. > > > Given that this report is isolated in a single file, we probably need > > > > I tested it on GCC 9 using modified make.cross from lkp and it triggers > > on one more file: > > > > drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp': > > ./include/linux/bitmap.h:601:18: error: array subscript [1, > > 288230376151711744] is outside array bounds of 'long unsigned int[1]' > > [-Werror=array-bounds] > > > > > to give the compiler some hints about the range of values passed to > > > bitmap_write() rather than suppressing the optimizations. > > > > OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it > > correctly, rather shuts up the compiler in cases like this one. > > > > I've been thinking of using __member_size() from fortify-string.h, we > > could probably optimize the object code even a bit more while silencing > > this warning. > > Adding Kees, maybe he'd like to participate in sorting this out as well. > > I'm trying to find all the pieces for this code, so I might be working > from the wrong version or something, but I think this is the change: > https://github.com/alobakin/linux/commit/66808fb20fed014a522b868322d54daef14a6bd8 This is the series: https://www.spinics.net/lists/kernel/msg4985590.html The relevant part of the function is: + offset = start % BITS_PER_LONG; + space = BITS_PER_LONG - offset; + fit = space >= nbits; // true if offset + nbits <= BITS_PER_LONG + index = BIT_WORD(start); + + map[index] &= XXX; + map[index] |= value << offset; + if (fit) + return; + + map[index + 1] = YYY; Some background for you: 'fit' means that the requested part of bitmap fits into a single machine word. For example, on 64-bit machine: DECLARE_BITMAP(map, 64); // unsigned long val[1]; bitmap_write(map, val, 60, 4) // fit == true bitmap_write(map, val, 60, 8) // fit == false It's possible that user may overrun the array boundary, like in the 2nd case, and compiler may correctly warn about it. But in this case... The code in question is: #define bitmap_set_value8(map, value, start) \ bitmap_write(map, value, start, BITS_PER_BYTE) #define BANK_SZ 8 for (i = 0; i < NBANK(chip); i++) bitmap_set_value8(val, value[i], i * BANK_SZ); Here nbits is always 8, and start is multiple of 8. With that, we're always writing into a single word (fit == true), and 'idx + 1' path is never hit. This makes me think that it the warning here is false positive. Is that correct? > and the induced warning is correctly analyzed in this thread (i.e. GCC > can't convince itself that it'll never reach the out of bounds access). > Does this work? > > - if (fit) > + if (fit || index + 1 >= __member_size(map)) > return; I already commented this in the other email in this thread - this would silence a true warning where people break the boundary: bitmap_write(map, val, 60, 8); And as far as I can see, __member_size() implies some runtime overhead, which I'd like to avoid. Thanks, Yury ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' 2023-11-06 16:31 ` Alexander Lobakin 2023-11-06 18:23 ` Andy Shevchenko @ 2023-11-07 13:22 ` Yury Norov 1 sibling, 0 replies; 15+ messages in thread From: Yury Norov @ 2023-11-07 13:22 UTC (permalink / raw) To: Alexander Lobakin Cc: Alexander Potapenko, Andy Shevchenko, Syed Nayyar Waris, kernel test robot, oe-kbuild-all, linux-kernel@vger.kernel.org On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote: > BTW, I have this in my inbox: > > From: Kernel Test Robot <lkp@intel.com> > Date: Tue, 17 Oct 2023 08:14:51 +0800 > > > tree: https://github.com/alobakin/linux pfcp > > head: 9183a3eb639912169a3d3e2be4f25556b465919b > > commit: c8a652cdcc0964510f108726b3da0784d1bc0cd2 [11/19] bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}() > > So it happened after I converted bitmap_{get,set}_value8() so that they > use bitmap_{read,write}(). > > > config: x86_64-randconfig-004-20231017 (https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/config) > > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce) [...] > > 643 | 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:21, > > from include/linux/ioport.h:13, > > from include/linux/acpi.h:12, > > from drivers/gpio/gpio-pca953x.c:11: > > drivers/gpio/gpio-pca953x.c:1032:17: note: while referencing 'val' It looks like a gcc-9 false-positive. I tried gcc-12 and gcc-13, and they both looks OK. Below is the fix that works for me. Can you please test the following patch and add it to your series? Thanks, Yury From 2a883221ddbd18796ddef0125e9e3022a56edd7b Mon Sep 17 00:00:00 2001 From: Yury Norov <yury.norov@gmail.com> Date: Tue, 7 Nov 2023 05:05:17 -0800 Subject: [PATCH] bitmap: suppress false-positive -Warray-bounds in bitmap_{read,write} bitmap_{read,write} conditionally accesses map[index + 1], carefully checking before that it's a safe dereference. But still, gcc-9 emits -Warray-bounds. Gcc-12 and gcc-13 are both OK with this code. So fix it for gcc-9 with OPTIMIZER_HIDE_VAR(). Reported-by: Alexander Lobakin <aleksander.lobakin@intel.com> Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/ Signed-off-by: Yury Norov <yury.norov@gmail.com> --- include/linux/bitmap.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 945a680816cc..0b1a07ff1080 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -592,6 +592,11 @@ static inline unsigned long bitmap_read(const unsigned long *map, if (unlikely(!nbits || nbits > BITS_PER_LONG)) return 0; +#if CONFIG_GCC_VERSION < 100000 + /* Suppress gcc-9 false-positive -Warray-bounds */ + OPTIMIZER_HIDE_VAR(map); +#endif + if (space >= nbits) return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits); @@ -634,6 +639,11 @@ static inline void bitmap_write(unsigned long *map, unsigned long value, fit = space >= nbits; index = BIT_WORD(start); +#if CONFIG_GCC_VERSION < 100000 + /* Suppress gcc-9 false-positive -Warray-bounds */ + OPTIMIZER_HIDE_VAR(map); +#endif + map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start)); map[index] |= value << offset; if (fit) -- 2.39.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-11-08 12:29 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-17 0:14 [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]' kernel test robot 2023-11-06 16:31 ` Alexander Lobakin 2023-11-06 18:23 ` Andy Shevchenko 2023-11-07 13:21 ` Alexander Lobakin 2023-11-07 16:33 ` Alexander Potapenko 2023-11-07 16:44 ` Alexander Lobakin 2023-11-07 17:24 ` Alexander Lobakin 2023-11-07 18:32 ` Yury Norov 2023-11-07 18:52 ` Alexander Lobakin 2023-11-07 19:24 ` Yury Norov 2023-11-08 10:07 ` Alexander Potapenko 2023-11-08 12:28 ` Alexander Lobakin 2023-11-07 23:25 ` Kees Cook 2023-11-08 0:48 ` Yury Norov 2023-11-07 13:22 ` Yury Norov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.