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

[Yury Norov <yury.norov@gmail.com>]

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