Re: kasan behavior when built with unsupported compiler

[Nikolay Borisov <n.borisov.lkml@gmail.com>]

On  9.03.2017 11:46, Andrey Ryabinin wrote:
> On 03/08/2017 11:10 AM, Nikolay Borisov wrote:
> 
>>
>> So apparently this is indeed a false positive, resulting from using the old 
>> compiler. I used the attached patch to verify it. 
>>
>> And what it prints is : 
>> [   17.184288] Assigned fbdev-blacklist.conff(ffff880001ea8020)20 whole object: ffff88006ae8fdb0 inode:ffff88006bff60d0
>> [   17.185808] Calling filldir with ffff88006ae8fdb0
>>
>> So the first line essentially happens when the object ffff88006ae8fdb0 is
>> being allocated and the second when it's used in filldir. The warning in 
>> ext4_ext_map_blocks doesn't trigger. However, if I remove the check for 
>> the value of ext4_global_pointer then I see multiple lines such as: 
>> [   17.386283] ext4_ext_map_blocks:freeing  pointer used in ext4_htree_store_dirent: ffff88006ae8fdb0 inode: ffff88006bff60d0
>> [   17.387601] Assigned fbdev-blacklist.conff(ffff880001eb3020)20 whole object: ffff88006ae8fdb0 inode:ffff88006bff60d0
>> [   17.388740] Calling filldir with ffff88006ae8fdb0
>>
>> so that same object was used right before it is allocated again in 
>> ext4_htree_store_dirent. And when you think about it it is logical since 
>> before filling in the dentry names in ext4_htree_store_dirent ext4 has to fetch the 
>> contents of the directory from disk.
>>
>> This leads me to believe that kasan is getting confused thinking that 
>> the object is being freed 
> 
> As I said before, this is *not* use-after-free. It's out-of-bounds access.
> No, kasan is not confused, it doesn't think that object is freed.
> Object is allocated and kasan see it as allocated object.
> The problem is that filldir reads past the end of that allocated object.
> 
> I don't see any sign that it's a false-positive.

Okay in that case I will continue digging. So the name is indeed housed
at the end of the struct fname. In ext4_htree_store_dirent this
structure seems allocated correctly sizeof(struct fname) + ent_name->len
+ 1;

Also the read should indeed be 20 bytes since the filename in question
fbdev-blacklist.conf is indeed 20 bytes. This implies that the 'namlen'
passed to copy_to_user is also correct. I guess I will have to look at
the generated assembly between the 2 gcc versions and see if anything
pops out.

> 
> 
>> AFTER being allocated in 
>> ext4_htree_store_dirent but testing shows it's being freed BEFORE. So 
>> I deem this a false positive, triggered by the compiler. 
>>
>>
>>