linux-hardening - 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]'

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e3b4b1a6-5383-4bfb-ba83-ab27d58dae29@intel.com>
Date: Tue, 7 Nov 2023 18:24:04 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Alexander Potapenko <glider@...gle.com>
CC: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Yury Norov
	<yury.norov@...il.com>, Syed Nayyar Waris <syednwaris@...il.com>, Kees Cook
	<keescook@...omium.org>, kernel test robot <lkp@...el.com>,
	<oe-kbuild-all@...ts.linux.dev>, <linux-hardening@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: 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]'

From: Alexander Lobakin <aleksander.lobakin@...el.com>
Date: Tue, 7 Nov 2023 17:44:00 +0100

> From: Alexander Potapenko <glider@...gle.com>
> Date: Tue, 7 Nov 2023 17:33:56 +0100
> 
>> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin
>> <aleksander.lobakin@...el.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);