Re: PAX: size overflow detected in function ext4_llseek fs/ext4/file.c:670

[Shawn C <citypw@gmail.com>]

On 05/12/2017 05:58 PM, PaX Team wrote:
> On 12 May 2017 at 1:02, Theodore Ts'o wrote:
> 
> [added Emese to CC and thus kept the whole mail for context even if
> i'm responding to some parts of it only]
> 
>> On Fri, May 12, 2017 at 12:10:04PM +0800, Shawn wrote:
>>>
>>> thanks for the reports, keep them coming . this is an interesting one,
>>> here's the code (same at both lines in ext4_seek_hole and ext4_seek_data):
>>>
>>> 670 »·······start = offset >> blkbits;
>>>
>>> in types this is
>>>
>>> u32 = long long >> int;
>>>
>>> since the maximum value was exceeded it means that offset (long long,
>>> 64 bits even on 32 bit archs) had a value that didn't fit u32 when right
>>> shifted. based on some light code reading, blkbits is between 10-16 on
>>> ext4 (EXT4_MIN_BLOCK_SIZE-EXT4_MAX_BLOCK_SIZE) so depending on what the
>>> actual block size of the underlying filesystem was, offset must have been
>>> bigger than 2^42-2^48 (4TB-256TB).
>>
>> Yes, this is indeed an interesting one.  I actually suspect that
>> offset was *negative*, and since start is a u32, this got translated
>> into a large number.
> 
> Shawn, can you do the printk instrumentation i suggested to print out
> the value of offset (and isize too while at it)?
>
I inserted the printk info in fs/ext4/file.c:677

00------------------
        printk(KERN_DEBUG"offset: %lld, isize: %lld, blkbits: %d\n",
offset, isize, blkbits);
11------------------

result:

offset: 105, isize: 19595264, blkbits: 12


> thing is, IIRC the C standard makes right shifting a negative value
> implementation defined (so excluding such values would be good for that
> reason alone) and i think gcc simply executes it as an arithmetic shift,
> i.e., the sign of the result is preserved and thus the size overflow
> checks should have detected a minimum violation, not the maximum one.
> 
> to tell for sure what check triggered exactly, i'd like to ask Shawn
> to execute
> 
> 	make fs/ext4/file.o EXTRA_CFLAGS="-fdump-tree-all -fdump-ipa-all"
> 
> and send us (Emese in particular) all the resulting files (fs/ext4/file.*)
> 
find the file here:
https://ufile.io/9ie40

Let me know if you need anything else.


regards
Shawn