Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1717312 > unrolled thread

Re: [PATCH 1/2] ext4: fix warning about stack corruption

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/2] ext4: fix warning about stack corruption Anton Blanchard <anton@ozlabs.org> - 2017-08-22 13:10 +0200
    Re: [PATCH 1/2] ext4: fix warning about stack corruption Arnd Bergmann <arnd@arndb.de> - 2017-08-22 14:00 +0200
      Re: [PATCH 1/2] ext4: fix warning about stack corruption Anton Blanchard <anton@ozlabs.org> - 2017-08-22 14:30 +0200

#1717312 — Re: [PATCH 1/2] ext4: fix warning about stack corruption

Hi Arnd,

> After commit 62d1034f53e3 ("fortify: use WARN instead of BUG for
> now"), we get a warning about possible stack overflow from a memcpy
> that was not strictly bounded to the size of the local variable:
> 
>     inlined from 'ext4_mb_seq_groups_show' at
> fs/ext4/mballoc.c:2322:2: include/linux/string.h:309:9: error:
> '__builtin_memcpy': writing between 161 and 1116 bytes into a region
> of size 160 overflows the destination [-Werror=stringop-overflow=]
> 
> We actually had a bug here that would have been found by the warning,
> but it was already fixed last year in commit 30a9d7afe70e ("ext4: fix
> stack memory corruption with 64k block size").
> 
> This replaces the fixed-length structure on the stack with a
> variable-length structure, using the correct upper bound that tells
> the compiler that everything is really fine here. I also change the
> loop count to check for the same upper bound for consistency, but the
> existing code is already correct here.
> 
> Note that while clang won't allow certain kinds of variable-length
> arrays in structures, this particular instance is fine, as the array
> is at the end of the structure, and the size is strictly bounded.

Unfortunately it doesn't appear to work, at least with ppc64le clang:

fs/ext4/mballoc.c:2303:17: error: fields must have a constant size: 'variable length array in structure' extension will never be supported
	ext4_grpblk_t counters[blocksize_bits + 2];

Anton

[toc] | [next] | [standalone]

#1717343

On Tue, Aug 22, 2017 at 1:08 PM, Anton Blanchard <anton@ozlabs.org> wrote:
> Hi Arnd,
>>
>> Note that while clang won't allow certain kinds of variable-length
>> arrays in structures, this particular instance is fine, as the array
>> is at the end of the structure, and the size is strictly bounded.
>
> Unfortunately it doesn't appear to work, at least with ppc64le clang:
>
> fs/ext4/mballoc.c:2303:17: error: fields must have a constant size: 'variable length array in structure' extension will never be supported
>         ext4_grpblk_t counters[blocksize_bits + 2];

My fix for this is in the ext4/dev branch in linux-next, I hope it still
makes it into v4.13.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1717364

> > Unfortunately it doesn't appear to work, at least with ppc64le
> > clang:
> >
> > fs/ext4/mballoc.c:2303:17: error: fields must have a constant size:
> > 'variable length array in structure' extension will never be
> > supported ext4_grpblk_t counters[blocksize_bits + 2];  
> 
> My fix for this is in the ext4/dev branch in linux-next, I hope it
> still makes it into v4.13.

Thanks Arnd, I see it now.

Anton

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web