Re: kasan behavior when built with unsupported compiler

[Nikolay Borisov]

On  7.03.2017 17:54, Dmitry Vyukov wrote:
> On Tue, Mar 7, 2017 at 4:35 PM, Nikolay Borisov
> <n.borisov.lkml@xxxxxxxxx> wrote:
>> Hello,
>>
>> I've been chasing a particular UAF as reported by kasan
>> (https://www.spinics.net/lists/kernel/msg2458136.html). However, one
>> thing which I took notice of rather lately is that I was building my
>> kernel with gcc 4.7.4 which is not supported by kasan as indicated by
>> the following string:
>>
>> scripts/Makefile.kasan:19: Cannot use CONFIG_KASAN:
>> -fsanitize=kernel-address is not supported by compiler
>>
>>
>> Nevertheless, the kernel compiles and when I boot it I see the kasan
>> splats as per the referenced thread. If, however, I build the kernel
>> with a newer compiler version 5.4.0 kasan no longer complains.
>>
>>
>> At this point I'm wondering whether the splats can be due to old
>> compiler being used e.g. false positives or are they genuine splats and
>> gcc 5 somehow obfuscates them ? Clearly despite the warning about not
>> being able to use CONFIG_KASAN it is still working since I'm seeing the
>> splats. Is this valid behavior ?
> 
> 
> Hi,
> 
> Re the message that kasan is not supported while it's still enabled in the end.
> I think it's an issue related to gcc plugins. Originally kasan was
> supported with 5.0+ thus the message. However, later we extended this
> support to 4.5+ with gcc plugins. However, that confusing message from
> build system was not fixed. So yes, it's confusing and it's something
> to fix, but mostly you can just ignore it.
> 
> Re false positives with 4.7. By default I would assume that it is true
> positive. Should be easy to check with manual printfs.

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 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. 



> 
> Re why 5.4 does not detect it. Difficult to say.
> If you confirm that it's a real bug and provide repro instructions,
> then I can recheck it with latest gcc. If it's a real bug and the
> latest gcc does not detect it, then we need to look more closely at
> it. I afraid 5.4 won't be fixed.
> It's also possible that it's a false positive in the old compiler (I
> think there were some bugs). If so, I would recommend switching to a
> newer compiler.
> 
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 68323e3da3fa..31f5153b3df4 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -429,6 +429,7 @@ void ext4_htree_free_dir_info(struct dir_private_info *p)
  * encrypted filename, while the htree will hold decrypted filename.
  * The decrypted filename is passed in via ent_name.  parameter.
  */
+void *global_ext4_pointer = NULL;
 int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
 			     __u32 minor_hash,
 			    struct ext4_dir_entry_2 *dirent,
@@ -454,7 +455,10 @@ int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
 	new_fn->file_type = dirent->file_type;
 	memcpy(new_fn->name, ent_name->name, ent_name->len);
 	new_fn->name[ent_name->len] = 0;
-
+	if (!strcmp(new_fn->name, "fbdev-blacklist.conf")) {
+		pr_info("Assigned %s(%p)%u whole object: %p inode:%p\n", ent_name->name, ent_name->name, ent_name->len, new_fn, file_inode(dir_file));
+		global_ext4_pointer = new_fn; 
+	}
 	while (*p) {
 		parent = *p;
 		fname = rb_entry(parent, struct fname, rb_hash);
@@ -507,6 +511,8 @@ static int call_filldir(struct file *file, struct dir_context *ctx,
 	}
 	ctx->pos = hash2pos(file, fname->hash, fname->minor_hash);
 	while (fname) {
+		if (fname == global_ext4_pointer)
+			pr_info("Calling filldir with %p\n", fname);
 		if (!dir_emit(ctx, fname->name,
 				fname->name_len,
 				fname->inode,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 2a2eef9c14e4..590a8cbb66b4 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4276,6 +4276,8 @@ static int get_implied_cluster_alloc(struct super_block *sb,
  *
  * return < 0, error case.
  */
+extern void* global_ext4_pointer;
+
 int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 			struct ext4_map_blocks *map, int flags)
 {
@@ -4616,6 +4618,9 @@ out:
 	map->m_len = allocated;
 out2:
 	ext4_ext_drop_refs(path);
+	if (path == global_ext4_pointer) {
+		pr_info("%s:freeing  pointer used in ext4_htree_store_dirent: %p inode: %p\n", __func__, path, inode);
+	}
 	kfree(path);
 
 	trace_ext4_ext_map_blocks_exit(inode, flags, map,