lkml.org 
[lkml]   [2023]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] lib: zstd: Fix -Wstringop-overflow warning
Date


> On Jan 13, 2023, at 4:48 PM, Kees Cook <keescook@chromium.org> wrote:
>
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
>
> |-------------------------------------------------------------------!
>
> 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

Thanks, I was able to repro it! I will merge this patch into my tree.

If you would like to submit the same patch upstream yourself, I will accept the PR, otherwise I can submit an upstream PR.

Just to be certain, this patch is to work around a shortcoming in -Wstringop-overflow, but the code was otherwise correct?

Reviewed-by: Nick Terrell <terrelln@fb.com <mailto:terrelln@fb.com>>

Best,
Nick Terrell

> --
> Kees Cook


\
 
 \ /
  Last update: 2023-03-26 23:41    [W:1.301 / U:1.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site