Re: [PATCH v2] lib: zstd: Fix -Wstringop-overflow warning

[Kees Cook <keescook@chromium.org>]

On Tue, Jan 10, 2023 at 11:10:08PM +0000, Nick Terrell wrote:
> 
> 
> > On Jan 4, 2023, at 1:20 PM, Kees Cook <keescook@chromium.org> wrote:
> > 
> > !-------------------------------------------------------------------|
> >  This Message Is From an External Sender
> > 
> > |-------------------------------------------------------------------!
> > 
> > Fix the following -Wstringop-overflow warning when building with GCC 11+:
> > 
> > lib/zstd/decompress/huf_decompress.c: In function ‘HUF_readDTableX2_wksp’:
> > lib/zstd/decompress/huf_decompress.c:700:5: warning: ‘HUF_fillDTableX2.constprop’ accessing 624 bytes in a region of size 52 [-Wstringop-overflow=]
> >  700 |     HUF_fillDTableX2(dt, maxTableLog,
> >      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  701 |                    wksp->sortedSymbol, sizeOfSort,
> >      |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  702 |                    wksp->rankStart0, wksp->rankVal, maxW,
> >      |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  703 |                    tableLog+1,
> >      |                    ~~~~~~~~~~~
> >  704 |                    wksp->calleeWksp, sizeof(wksp->calleeWksp) / sizeof(U32));
> >      |
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > lib/zstd/decompress/huf_decompress.c:700:5: note: referencing argument 6 of type ‘U32 (*)[13]’ {aka ‘unsigned int (*)[13]’}
> > lib/zstd/decompress/huf_decompress.c:571:13: note: in a call to function ‘HUF_fillDTableX2.constprop’
> >  571 | static void HUF_fillDTableX2(HUF_DEltX2* DTable, const U32 targetLog,
> >      |             ^~~~~~~~~~~~~~~~
> > 
> > by using pointer notation instead of array notation.
> > 
> > This is one of the last remaining warnings to be fixed before globally
> > enabling -Wstringop-overflow.
> 
> The patch looks correct to me, thanks for reviving it. But, I was attempting to reproduce the issue,
> so I could better understand what's going on, and I wasn't able to reproduce it myself.
> 
> To attempt to reproduce, I applied this patch
> 
> ---
> diff --git a/lib/zstd/Makefile b/lib/zstd/Makefile
> index 20f08c644b71..190d3d5ab4be 100644
> --- a/lib/zstd/Makefile
> +++ b/lib/zstd/Makefile
> @@ -12,6 +12,8 @@ obj-$(CONFIG_ZSTD_COMPRESS) += zstd_compress.o
>  obj-$(CONFIG_ZSTD_DECOMPRESS) += zstd_decompress.o
>  obj-$(CONFIG_ZSTD_COMMON) += zstd_common.o
>  
> +ccflags-y := -Wstringop-overflow=4 -Werror
> +
>  zstd_compress-y := \
>                 zstd_compress_module.o \
>                 compress/fse_compress.o \
> ---
> 
> Then compiled on x86-64 with gcc 12.2.0 on tag v6.2-rc3. I saw no errors.
> I also tried with just `-Wstringop-overflow`, and on upstream zstd. I tried to
> make a minimal reproducer on godbolt, so I could see if it was the gcc version,
> but wasn't able to make it fail with any of them https://gcc.godbolt.org/z/Exzq9arMr.
> 
> Could you please tell me how to reproduce this warning?

I saw it like so with next-20230113 on x86_64:

$ gcc --version
gcc (Ubuntu 12.2.0-3ubuntu1) 12.2.0
...
$ make KCFLAGS=-Wstringop-overflow allmodconfig lib/zstd/decompress/huf_decompress.o

-- 
Kees Cook