ksmbd: fix parent dentry lifetime in directory enumeration
Subsystems (3)
PATCH 1/1 ksmbd: fix parent dentry lifetime in directory enumeration
ksmbd_populate_dot_dotdot_entries() builds the ".." entry from the open directory's d_parent and passes that pointer to ksmbd_vfs_fill_dentry_attrs() without taking a reference. That pointer is only valid while the directory stays under this parent. If a concurrent rename moves the directory elsewhere, the old parent loses its last reference; a following rmdir then turns it into a negative dentry with d_inode cleared, and after the RCU grace period the dentry itself is freed. The worker still holds the raw pointer and dereferences d_inode through vfs_getattr() and vfs_getxattr().
A remote SMB client, authenticated or guest, triggers this by listing a directory while that directory is moved under a new parent and the old parent is deleted. It can be done by the client itself with write access or by other concurrent writers, triggering null-ptr-deref and slab-use-after-free on a KASAN build.
KASAN: null-ptr-deref in range
[0x0000000000000000-0x0000000000000007]
Workqueue: ksmbd-io handle_ksmbd_work
RIP: 0010:xattr_permission+0x9d/0x160
Call Trace:
vfs_getxattr+0x9c/0x1b0
ksmbd_vfs_getxattr+0x82/0x100
ksmbd_vfs_get_dos_attrib_xattr+0x7f/0x130
ksmbd_vfs_fill_dentry_attrs+0x435/0x650
ksmbd_populate_dot_dotdot_entries+0x25e/0x630
smb2_query_dir+0xcf9/0x1a20
handle_ksmbd_work+0x5f1/0x1c80
process_one_work+0x90b/0x19d0Fix by using the dentry reference counting APIs: take the parent with dget_parent(), take a matching dget() on ".", and dput() the dentry once ksmbd_vfs_fill_dentry_attrs() has returned or the pattern does not match.
--- fs/smb/server/smb_common.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
fs/smb/server/smb_common.c +4 -2
@@ -478,16 +478,17 @@ int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level,if (i ==0 ) { d_info->name ="." ; d_info->name_len =1 ; - dentry = dir->filp->f_path.dentry; + dentry =dget (dir->filp->f_path.dentry); }else { d_info->name =".." ; d_info->name_len =2 ; - dentry = dir->filp->f_path.dentry->d_parent; + dentry =dget_parent (dir->filp->f_path.dentry); }if (!match_pattern (d_info->name, d_info->name_len, search_pattern)) { dir->dot_dotdot[i] =1 ; +dput (dentry);continue ; }
@@ -496,6 +497,7 @@ int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, idmap, dentry, &ksmbd_kstat); +dput (dentry);if (rc)break ;
Signature · 1 line
2.55.0
Quoted 36 lines · Cen Zhang (Microsoft)
ksmbd_populate_dot_dotdot_entries()builds the ".." entry from the open directory'sd_parentand passes that pointer toksmbd_vfs_fill_dentry_attrs()without taking a reference. That pointer is only valid while the directory stays under this parent. If a concurrent rename moves the directory elsewhere, the old parent loses its last reference; a following rmdir then turns it into a negative dentry withd_inodecleared, and after the RCU grace period the dentry itself is freed. The worker still holds the raw pointer and dereferencesd_inodethroughvfs_getattr()andvfs_getxattr().A remote SMB client, authenticated or guest, triggers this by listing a directory while that directory is moved under a new parent and the old parent is deleted. It can be done by the client itself with write access or by other concurrent writers, triggering null-ptr-deref and slab-use-after-free on a KASAN build.
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] Workqueue: ksmbd-io handle_ksmbd_work RIP: 0010:xattr_permission+0x9d/0x160 Call Trace: vfs_getxattr+0x9c/0x1b0 ksmbd_vfs_getxattr+0x82/0x100 ksmbd_vfs_get_dos_attrib_xattr+0x7f/0x130 ksmbd_vfs_fill_dentry_attrs+0x435/0x650 ksmbd_populate_dot_dotdot_entries+0x25e/0x630 smb2_query_dir+0xcf9/0x1a20 handle_ksmbd_work+0x5f1/0x1c80 process_one_work+0x90b/0x19d0Fix by using the dentry reference counting APIs: take the parent with
dget_parent(), take a matchingdget()on ".", anddput()the dentry onceksmbd_vfs_fill_dentry_attrs()has returned or the pattern does not match.
Quoted 4 lines
Fixes e2f34481b24d ("cifsd: add server-side procedures for SMB3")Signed-off-by Cen Zhang (Microsoft) <[email protected]>
Applied it to #ksmbd-for-next.
Thanks!