Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

cifs: open files should not hold ref on superblock

Today whenever we deal with a file, in addition to holding
a reference on the dentry, we also get a reference on the
superblock. This happens in two cases:
1. when a new cinode is allocated
2. when an oplock break is being processed

The reasoning for holding the superblock ref was to make sure
that when umount happens, if there are users of inodes and
dentries, it does not try to clean them up and wait for the
last ref to superblock to be dropped by last of such users.

But the side effect of doing that is that umount silently drops
a ref on the superblock and we could have deferred closes and
lease breaks still holding these refs.

Ideally, we should ensure that all of these users of inodes and
dentries are cleaned up at the time of umount, which is what this
code is doing.

This code change allows these code paths to use a ref on the
dentry (and hence the inode). That way, umount is
ensured to clean up SMB client resources when it's the last
ref on the superblock (For ex: when same objects are shared).

The code change also moves the call to close all the files in
deferred close list to the umount code path. It also waits for
oplock_break workers to be flushed before calling
kill_anon_super (which eventually frees up those objects).

Fixes: 24261fc23db9 ("cifs: delay super block destruction until all cifsFileInfo objects are gone")
Fixes: 705c79101ccf ("smb: client: fix use-after-free in cifs_oplock_break")
Cc: <stable@vger.kernel.org>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>

authored by

Shyam Prasad N and committed by
Steve French
340cea84 26bc83b8

+50 -13
+5 -2
fs/smb/client/cifsfs.c
··· 332 332 333 333 /* 334 334 * We need to release all dentries for the cached directories 335 - * before we kill the sb. 335 + * and close all deferred file handles before we kill the sb. 336 336 */ 337 337 if (cifs_sb->root) { 338 338 close_all_cached_dirs(cifs_sb); 339 + cifs_close_all_deferred_files_sb(cifs_sb); 340 + 341 + /* Wait for all pending oplock breaks to complete */ 342 + flush_workqueue(cifsoplockd_wq); 339 343 340 344 /* finally release root dentry */ 341 345 dput(cifs_sb->root); ··· 872 868 spin_unlock(&tcon->tc_lock); 873 869 spin_unlock(&cifs_tcp_ses_lock); 874 870 875 - cifs_close_all_deferred_files(tcon); 876 871 /* cancel_brl_requests(tcon); */ /* BB mark all brl mids as exiting */ 877 872 /* cancel_notify_requests(tcon); */ 878 873 if (tcon->ses && tcon->ses->server) {
+1
fs/smb/client/cifsproto.h
··· 261 261 262 262 void cifs_close_all_deferred_files(struct cifs_tcon *tcon); 263 263 264 + void cifs_close_all_deferred_files_sb(struct cifs_sb_info *cifs_sb); 264 265 void cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon, 265 266 struct dentry *dentry); 266 267
-11
fs/smb/client/file.c
··· 711 711 mutex_init(&cfile->fh_mutex); 712 712 spin_lock_init(&cfile->file_info_lock); 713 713 714 - cifs_sb_active(inode->i_sb); 715 - 716 714 /* 717 715 * If the server returned a read oplock and we have mandatory brlocks, 718 716 * set oplock level to None. ··· 765 767 struct inode *inode = d_inode(cifs_file->dentry); 766 768 struct cifsInodeInfo *cifsi = CIFS_I(inode); 767 769 struct cifsLockInfo *li, *tmp; 768 - struct super_block *sb = inode->i_sb; 769 770 770 771 /* 771 772 * Delete any outstanding lock records. We'll lose them when the file ··· 782 785 783 786 cifs_put_tlink(cifs_file->tlink); 784 787 dput(cifs_file->dentry); 785 - cifs_sb_deactive(sb); 786 788 kfree(cifs_file->symlink_target); 787 789 kfree(cifs_file); 788 790 } ··· 3159 3163 __u64 persistent_fid, volatile_fid; 3160 3164 __u16 net_fid; 3161 3165 3162 - /* 3163 - * Hold a reference to the superblock to prevent it and its inodes from 3164 - * being freed while we are accessing cinode. Otherwise, _cifsFileInfo_put() 3165 - * may release the last reference to the sb and trigger inode eviction. 3166 - */ 3167 - cifs_sb_active(sb); 3168 3166 wait_on_bit(&cinode->flags, CIFS_INODE_PENDING_WRITERS, 3169 3167 TASK_UNINTERRUPTIBLE); 3170 3168 ··· 3243 3253 cifs_put_tlink(tlink); 3244 3254 out: 3245 3255 cifs_done_oplock_break(cinode); 3246 - cifs_sb_deactive(sb); 3247 3256 } 3248 3257 3249 3258 static int cifs_swap_activate(struct swap_info_struct *sis,
+42
fs/smb/client/misc.c
··· 28 28 #include "fs_context.h" 29 29 #include "cached_dir.h" 30 30 31 + struct tcon_list { 32 + struct list_head entry; 33 + struct cifs_tcon *tcon; 34 + }; 35 + 31 36 /* The xid serves as a useful identifier for each incoming vfs request, 32 37 in a similar way to the mid which is useful to track each sent smb, 33 38 and CurrentXid can also provide a running counter (although it ··· 555 550 list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) { 556 551 _cifsFileInfo_put(tmp_list->cfile, true, false); 557 552 list_del(&tmp_list->list); 553 + kfree(tmp_list); 554 + } 555 + } 556 + 557 + void cifs_close_all_deferred_files_sb(struct cifs_sb_info *cifs_sb) 558 + { 559 + struct rb_root *root = &cifs_sb->tlink_tree; 560 + struct rb_node *node; 561 + struct cifs_tcon *tcon; 562 + struct tcon_link *tlink; 563 + struct tcon_list *tmp_list, *q; 564 + LIST_HEAD(tcon_head); 565 + 566 + spin_lock(&cifs_sb->tlink_tree_lock); 567 + for (node = rb_first(root); node; node = rb_next(node)) { 568 + tlink = rb_entry(node, struct tcon_link, tl_rbnode); 569 + tcon = tlink_tcon(tlink); 570 + if (IS_ERR(tcon)) 571 + continue; 572 + tmp_list = kmalloc_obj(struct tcon_list, GFP_ATOMIC); 573 + if (tmp_list == NULL) 574 + break; 575 + tmp_list->tcon = tcon; 576 + /* Take a reference on tcon to prevent it from being freed */ 577 + spin_lock(&tcon->tc_lock); 578 + ++tcon->tc_count; 579 + trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count, 580 + netfs_trace_tcon_ref_get_close_defer_files); 581 + spin_unlock(&tcon->tc_lock); 582 + list_add_tail(&tmp_list->entry, &tcon_head); 583 + } 584 + spin_unlock(&cifs_sb->tlink_tree_lock); 585 + 586 + list_for_each_entry_safe(tmp_list, q, &tcon_head, entry) { 587 + cifs_close_all_deferred_files(tmp_list->tcon); 588 + list_del(&tmp_list->entry); 589 + cifs_put_tcon(tmp_list->tcon, netfs_trace_tcon_ref_put_close_defer_files); 558 590 kfree(tmp_list); 559 591 } 560 592 }
+2
fs/smb/client/trace.h
··· 176 176 EM(netfs_trace_tcon_ref_get_cached_laundromat, "GET Ch-Lau") \ 177 177 EM(netfs_trace_tcon_ref_get_cached_lease_break, "GET Ch-Lea") \ 178 178 EM(netfs_trace_tcon_ref_get_cancelled_close, "GET Cn-Cls") \ 179 + EM(netfs_trace_tcon_ref_get_close_defer_files, "GET Cl-Def") \ 179 180 EM(netfs_trace_tcon_ref_get_dfs_refer, "GET DfsRef") \ 180 181 EM(netfs_trace_tcon_ref_get_find, "GET Find ") \ 181 182 EM(netfs_trace_tcon_ref_get_find_sess_tcon, "GET FndSes") \ ··· 188 187 EM(netfs_trace_tcon_ref_put_cancelled_close, "PUT Cn-Cls") \ 189 188 EM(netfs_trace_tcon_ref_put_cancelled_close_fid, "PUT Cn-Fid") \ 190 189 EM(netfs_trace_tcon_ref_put_cancelled_mid, "PUT Cn-Mid") \ 190 + EM(netfs_trace_tcon_ref_put_close_defer_files, "PUT Cl-Def") \ 191 191 EM(netfs_trace_tcon_ref_put_mnt_ctx, "PUT MntCtx") \ 192 192 EM(netfs_trace_tcon_ref_put_dfs_refer, "PUT DfsRfr") \ 193 193 EM(netfs_trace_tcon_ref_put_reconnect_server, "PUT Reconn") \