ceph: fix use-after-free in __ceph_remove_cap()
authorLuis Henriques <lhenriques@suse.com>
Fri, 25 Oct 2019 13:05:24 +0000 (14:05 +0100)
committerIlya Dryomov <idryomov@gmail.com>
Tue, 29 Oct 2019 21:29:51 +0000 (22:29 +0100)
KASAN reports a use-after-free when running xfstest generic/531, with the
following trace:

[  293.903362]  kasan_report+0xe/0x20
[  293.903365]  rb_erase+0x1f/0x790
[  293.903370]  __ceph_remove_cap+0x201/0x370
[  293.903375]  __ceph_remove_caps+0x4b/0x70
[  293.903380]  ceph_evict_inode+0x4e/0x360
[  293.903386]  evict+0x169/0x290
[  293.903390]  __dentry_kill+0x16f/0x250
[  293.903394]  dput+0x1c6/0x440
[  293.903398]  __fput+0x184/0x330
[  293.903404]  task_work_run+0xb9/0xe0
[  293.903410]  exit_to_usermode_loop+0xd3/0xe0
[  293.903413]  do_syscall_64+0x1a0/0x1c0
[  293.903417]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

This happens because __ceph_remove_cap() may queue a cap release
(__ceph_queue_cap_release) which can be scheduled before that cap is
removed from the inode list with

rb_erase(&cap->ci_node, &ci->i_caps);

And, when this finally happens, the use-after-free will occur.

This can be fixed by removing the cap from the inode list before being
removed from the session list, and thus eliminating the risk of an UAF.

Cc: stable@vger.kernel.org
Signed-off-by: Luis Henriques <lhenriques@suse.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
fs/ceph/caps.c

index d3b9c9d5c1bd242170d671521d91ddb6cb90a0ac..f5a38910a82bfb94d71ceab28af196b3d0165c6f 100644 (file)
@@ -1058,6 +1058,11 @@ void __ceph_remove_cap(struct ceph_cap *cap, bool queue_release)
 
        dout("__ceph_remove_cap %p from %p\n", cap, &ci->vfs_inode);
 
+       /* remove from inode's cap rbtree, and clear auth cap */
+       rb_erase(&cap->ci_node, &ci->i_caps);
+       if (ci->i_auth_cap == cap)
+               ci->i_auth_cap = NULL;
+
        /* remove from session list */
        spin_lock(&session->s_cap_lock);
        if (session->s_cap_iterator == cap) {
@@ -1091,11 +1096,6 @@ void __ceph_remove_cap(struct ceph_cap *cap, bool queue_release)
 
        spin_unlock(&session->s_cap_lock);
 
-       /* remove from inode list */
-       rb_erase(&cap->ci_node, &ci->i_caps);
-       if (ci->i_auth_cap == cap)
-               ci->i_auth_cap = NULL;
-
        if (removed)
                ceph_put_cap(mdsc, cap);