KASAN: use-after-free Read in do_mount

KASAN: use-after-free Read in do_mount

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:    311ef88a Add linux-next specific files for 20191004
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=16ce4899600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=db2e4361e48662f4
dashboard link: https://syzkaller.appspot.com/bug?extid=da4f525235510683d855
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17b672b9600000

The bug was bisected to:

commit 9d14545b05f9eed69fbd4af14b927a462324ea19
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Fri Aug 30 16:12:15 2019 +0000

     Merge branch 'limits' of https://github.com/deepa-hub/vfs into y2038

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=16eeee2b600000
final crash:    https://syzkaller.appspot.com/x/report.txt?x=15eeee2b600000
console output: https://syzkaller.appspot.com/x/log.txt?x=11eeee2b600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+da4f525235510683d855@syzkaller.appspotmail.com
Fixes: 9d14545b05f9 ("Merge branch 'limits' of  
https://github.com/deepa-hub/vfs into y2038")

==================================================================
BUG: KASAN: use-after-free in do_new_mount_fc fs/namespace.c:2773 [inline]
BUG: KASAN: use-after-free in do_new_mount fs/namespace.c:2825 [inline]
BUG: KASAN: use-after-free in do_mount+0x1b5f/0x1d10 fs/namespace.c:3143
Read of size 8 at addr ffff88809a505b28 by task syz-executor.4/13945

CPU: 1 PID: 13945 Comm: syz-executor.4 Not tainted 5.4.0-rc1-next-20191004  
#0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
  __kasan_report.cold+0x1b/0x41 mm/kasan/report.c:506
  kasan_report+0x12/0x20 mm/kasan/common.c:634
  __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
  do_new_mount_fc fs/namespace.c:2773 [inline]
  do_new_mount fs/namespace.c:2825 [inline]
  do_mount+0x1b5f/0x1d10 fs/namespace.c:3143
  ksys_mount+0xdb/0x150 fs/namespace.c:3352
  __do_sys_mount fs/namespace.c:3366 [inline]
  __se_sys_mount fs/namespace.c:3363 [inline]
  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3363
  do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459a59
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f1c10834c78 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 0000000000459a59
RDX: 0000000020000a40 RSI: 00000000200005c0 RDI: 0000000000000000
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f1c108356d4
R13: 00000000004c6291 R14: 00000000004db2f8 R15: 00000000ffffffff

Allocated by task 13945:
  save_stack+0x23/0x90 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_kmalloc mm/kasan/common.c:510 [inline]
  __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:483
  kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:518
  slab_post_alloc_hook mm/slab.h:584 [inline]
  slab_alloc mm/slab.c:3319 [inline]
  kmem_cache_alloc+0x121/0x710 mm/slab.c:3483
  kmem_cache_zalloc include/linux/slab.h:680 [inline]
  alloc_vfsmnt+0x28/0x680 fs/namespace.c:177
  vfs_create_mount+0x96/0x500 fs/namespace.c:940
  do_new_mount_fc fs/namespace.c:2763 [inline]
  do_new_mount fs/namespace.c:2825 [inline]
  do_mount+0x17ae/0x1d10 fs/namespace.c:3143
  ksys_mount+0xdb/0x150 fs/namespace.c:3352
  __do_sys_mount fs/namespace.c:3366 [inline]
  __se_sys_mount fs/namespace.c:3363 [inline]
  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3363
  do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 16:
  save_stack+0x23/0x90 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  kasan_set_free_info mm/kasan/common.c:332 [inline]
  __kasan_slab_free+0x102/0x150 mm/kasan/common.c:471
  kasan_slab_free+0xe/0x10 mm/kasan/common.c:480
  __cache_free mm/slab.c:3425 [inline]
  kmem_cache_free+0x86/0x320 mm/slab.c:3693
  free_vfsmnt+0x6f/0x90 fs/namespace.c:554
  delayed_free_vfsmnt+0x16/0x20 fs/namespace.c:559
  __rcu_reclaim kernel/rcu/rcu.h:222 [inline]
  rcu_do_batch kernel/rcu/tree.c:2157 [inline]
  rcu_core+0x581/0x1560 kernel/rcu/tree.c:2377
  rcu_core_si+0x9/0x10 kernel/rcu/tree.c:2386
  __do_softirq+0x262/0x98c kernel/softirq.c:292

The buggy address belongs to the object at ffff88809a505b00
  which belongs to the cache mnt_cache of size 312
The buggy address is located 40 bytes inside of
  312-byte region [ffff88809a505b00, ffff88809a505c38)
The buggy address belongs to the page:
page:ffffea0002694140 refcount:1 mapcount:0 mapping:ffff8880aa5a88c0  
index:0x0
flags: 0x1fffc0000000200(slab)
raw: 01fffc0000000200 ffffea000274afc8 ffffea0002982688 ffff8880aa5a88c0
raw: 0000000000000000 ffff88809a505080 000000010000000a 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff88809a505a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff88809a505a80: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc
> ffff88809a505b00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                   ^
  ffff88809a505b80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff88809a505c00: fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc fc
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

[PATCH] fs/namespace.c: fix use-after-free of mount in mnt_warn_timestamp_expiry()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

After do_add_mount() returns success, the caller doesn't hold a
reference to the 'struct mount' anymore.  So it's invalid to access it
in mnt_warn_timestamp_expiry().

Fix it by instead passing the super_block and the mnt_flags.  It's safe
to access the super_block because it's pinned by fs_context::root.

Reported-by: syzbot+da4f525235510683d855@syzkaller.appspotmail.com
Fixes: f8b92ba67c5d ("mount: Add mount warning for impending timestamp expiry")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/namespace.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index fe0e9e1410fe..7ef8edaaed69 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2466,12 +2466,11 @@ static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags)
 	unlock_mount_hash();
 }
 
-static void mnt_warn_timestamp_expiry(struct path *mountpoint, struct vfsmount *mnt)
+static void mnt_warn_timestamp_expiry(struct path *mountpoint,
+				      struct super_block *sb, int mnt_flags)
 {
-	struct super_block *sb = mnt->mnt_sb;
-
-	if (!__mnt_is_readonly(mnt) &&
-	   (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
+	if (!(mnt_flags & MNT_READONLY) && !sb_rdonly(sb) &&
+	    (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
 		char *buf = (char *)__get_free_page(GFP_KERNEL);
 		char *mntpath = buf ? d_path(mountpoint, buf, PAGE_SIZE) : ERR_PTR(-ENOMEM);
 		struct tm tm;
@@ -2512,7 +2511,7 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags)
 		set_mount_attributes(mnt, mnt_flags);
 	up_write(&sb->s_umount);
 
-	mnt_warn_timestamp_expiry(path, &mnt->mnt);
+	mnt_warn_timestamp_expiry(path, sb, mnt_flags);
 
 	return ret;
 }
@@ -2555,7 +2554,7 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
 		up_write(&sb->s_umount);
 	}
 
-	mnt_warn_timestamp_expiry(path, &mnt->mnt);
+	mnt_warn_timestamp_expiry(path, sb, mnt_flags);
 
 	put_fs_context(fc);
 	return err;
@@ -2770,7 +2769,7 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
 		return error;
 	}
 
-	mnt_warn_timestamp_expiry(mountpoint, mnt);
+	mnt_warn_timestamp_expiry(mountpoint, sb, mnt_flags);
 
 	return error;
 }
-- 
2.23.0

Re: [PATCH] fs/namespace.c: fix use-after-free of mount in mnt_warn_timestamp_expiry()

[Deepa Dinamani]

Thanks for the fix.

Would it be better to move the check and warning to a place where the
access is still safe?

-Deepa

> On Oct 9, 2019, at 12:19 AM, Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>


On Wed, Oct 9, 2019 at 12:19 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> After do_add_mount() returns success, the caller doesn't hold a
> reference to the 'struct mount' anymore.  So it's invalid to access it
> in mnt_warn_timestamp_expiry().
>
> Fix it by instead passing the super_block and the mnt_flags.  It's safe
> to access the super_block because it's pinned by fs_context::root.
>
> Reported-by: syzbot+da4f525235510683d855@syzkaller.appspotmail.com
> Fixes: f8b92ba67c5d ("mount: Add mount warning for impending timestamp expiry")
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/namespace.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index fe0e9e1410fe..7ef8edaaed69 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2466,12 +2466,11 @@ static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags)
>         unlock_mount_hash();
>  }
>
> -static void mnt_warn_timestamp_expiry(struct path *mountpoint, struct vfsmount *mnt)
> +static void mnt_warn_timestamp_expiry(struct path *mountpoint,
> +                                     struct super_block *sb, int mnt_flags)
>  {
> -       struct super_block *sb = mnt->mnt_sb;
> -
> -       if (!__mnt_is_readonly(mnt) &&
> -          (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
> +       if (!(mnt_flags & MNT_READONLY) && !sb_rdonly(sb) &&
> +           (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
>                 char *buf = (char *)__get_free_page(GFP_KERNEL);
>                 char *mntpath = buf ? d_path(mountpoint, buf, PAGE_SIZE) : ERR_PTR(-ENOMEM);
>                 struct tm tm;
> @@ -2512,7 +2511,7 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags)
>                 set_mount_attributes(mnt, mnt_flags);
>         up_write(&sb->s_umount);
>
> -       mnt_warn_timestamp_expiry(path, &mnt->mnt);
> +       mnt_warn_timestamp_expiry(path, sb, mnt_flags);
>
>         return ret;
>  }
> @@ -2555,7 +2554,7 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
>                 up_write(&sb->s_umount);
>         }
>
> -       mnt_warn_timestamp_expiry(path, &mnt->mnt);
> +       mnt_warn_timestamp_expiry(path, sb, mnt_flags);
>
>         put_fs_context(fc);
>         return err;
> @@ -2770,7 +2769,7 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
>                 return error;
>         }
>
> -       mnt_warn_timestamp_expiry(mountpoint, mnt);
> +       mnt_warn_timestamp_expiry(mountpoint, sb, mnt_flags);
>
>         return error;
>  }
> --
> 2.23.0
>

Re: [PATCH] fs/namespace.c: fix use-after-free of mount in mnt_warn_timestamp_expiry()

[Eric Biggers]

On Sun, Oct 13, 2019 at 07:04:10PM -0700, Deepa Dinamani wrote:
> Thanks for the fix.
> 
> Would it be better to move the check and warning to a place where the
> access is still safe?
> 
> -Deepa

True, we could just do

diff --git a/fs/namespace.c b/fs/namespace.c
index fe0e9e1410fe..9f2ceb542f05 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2764,14 +2764,14 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
 	if (IS_ERR(mnt))
 		return PTR_ERR(mnt);
 
+	mnt_warn_timestamp_expiry(mountpoint, mnt);
+
 	error = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
 	if (error < 0) {
 		mntput(mnt);
 		return error;
 	}
 
-	mnt_warn_timestamp_expiry(mountpoint, mnt);
-
 	return error;
 }

But then the warning ("Mounted %s file system ...") is printed even if
do_add_mount() fails so nothing was actually mounted.

Though, it's just a warning message and I think failures here are rare, so maybe
we don't care.  Al, what do you think?

- Eric