Skip to content
/ linux Public

Commit f655467

Browse files
sprasad-microsoftgregkh
authored andcommitted
cifs: open files should not hold ref on superblock
[ Upstream commit 340cea8 ] 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: 24261fc ("cifs: delay super block destruction until all cifsFileInfo objects are gone") Fixes: 705c791 ("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> [ replaced kmalloc_obj() with kmalloc(sizeof(...)) ] Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 6f50204 commit f655467

File tree

5 files changed

+50
-13
lines changed

5 files changed

+50
-13
lines changed

fs/smb/client/cifsfs.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,10 +330,14 @@ static void cifs_kill_sb(struct super_block *sb)
330330

331331
/*
332332
* We need to release all dentries for the cached directories
333-
* before we kill the sb.
333+
* and close all deferred file handles before we kill the sb.
334334
*/
335335
if (cifs_sb->root) {
336336
close_all_cached_dirs(cifs_sb);
337+
cifs_close_all_deferred_files_sb(cifs_sb);
338+
339+
/* Wait for all pending oplock breaks to complete */
340+
flush_workqueue(cifsoplockd_wq);
337341

338342
/* finally release root dentry */
339343
dput(cifs_sb->root);
@@ -864,7 +868,6 @@ static void cifs_umount_begin(struct super_block *sb)
864868
spin_unlock(&tcon->tc_lock);
865869
spin_unlock(&cifs_tcp_ses_lock);
866870

867-
cifs_close_all_deferred_files(tcon);
868871
/* cancel_brl_requests(tcon); */ /* BB mark all brl mids as exiting */
869872
/* cancel_notify_requests(tcon); */
870873
if (tcon->ses && tcon->ses->server) {

fs/smb/client/cifsproto.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ extern void cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode);
313313

314314
extern void cifs_close_all_deferred_files(struct cifs_tcon *cifs_tcon);
315315

316+
void cifs_close_all_deferred_files_sb(struct cifs_sb_info *cifs_sb);
316317
void cifs_close_deferred_file_under_dentry(struct cifs_tcon *cifs_tcon,
317318
struct dentry *dentry);
318319

fs/smb/client/file.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,6 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
704704
mutex_init(&cfile->fh_mutex);
705705
spin_lock_init(&cfile->file_info_lock);
706706

707-
cifs_sb_active(inode->i_sb);
708-
709707
/*
710708
* If the server returned a read oplock and we have mandatory brlocks,
711709
* set oplock level to None.
@@ -760,7 +758,6 @@ static void cifsFileInfo_put_final(struct cifsFileInfo *cifs_file)
760758
struct inode *inode = d_inode(cifs_file->dentry);
761759
struct cifsInodeInfo *cifsi = CIFS_I(inode);
762760
struct cifsLockInfo *li, *tmp;
763-
struct super_block *sb = inode->i_sb;
764761

765762
/*
766763
* Delete any outstanding lock records. We'll lose them when the file
@@ -778,7 +775,6 @@ static void cifsFileInfo_put_final(struct cifsFileInfo *cifs_file)
778775

779776
cifs_put_tlink(cifs_file->tlink);
780777
dput(cifs_file->dentry);
781-
cifs_sb_deactive(sb);
782778
kfree(cifs_file->symlink_target);
783779
kfree(cifs_file);
784780
}
@@ -3150,12 +3146,6 @@ void cifs_oplock_break(struct work_struct *work)
31503146
__u64 persistent_fid, volatile_fid;
31513147
__u16 net_fid;
31523148

3153-
/*
3154-
* Hold a reference to the superblock to prevent it and its inodes from
3155-
* being freed while we are accessing cinode. Otherwise, _cifsFileInfo_put()
3156-
* may release the last reference to the sb and trigger inode eviction.
3157-
*/
3158-
cifs_sb_active(sb);
31593149
wait_on_bit(&cinode->flags, CIFS_INODE_PENDING_WRITERS,
31603150
TASK_UNINTERRUPTIBLE);
31613151

@@ -3228,7 +3218,6 @@ void cifs_oplock_break(struct work_struct *work)
32283218
cifs_put_tlink(tlink);
32293219
out:
32303220
cifs_done_oplock_break(cinode);
3231-
cifs_sb_deactive(sb);
32323221
}
32333222

32343223
static int cifs_swap_activate(struct swap_info_struct *sis,

fs/smb/client/misc.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@
2727
#include "fs_context.h"
2828
#include "cached_dir.h"
2929

30+
struct tcon_list {
31+
struct list_head entry;
32+
struct cifs_tcon *tcon;
33+
};
34+
3035
/* The xid serves as a useful identifier for each incoming vfs request,
3136
in a similar way to the mid which is useful to track each sent smb,
3237
and CurrentXid can also provide a running counter (although it
@@ -833,6 +838,43 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon)
833838
}
834839
}
835840

841+
void cifs_close_all_deferred_files_sb(struct cifs_sb_info *cifs_sb)
842+
{
843+
struct rb_root *root = &cifs_sb->tlink_tree;
844+
struct rb_node *node;
845+
struct cifs_tcon *tcon;
846+
struct tcon_link *tlink;
847+
struct tcon_list *tmp_list, *q;
848+
LIST_HEAD(tcon_head);
849+
850+
spin_lock(&cifs_sb->tlink_tree_lock);
851+
for (node = rb_first(root); node; node = rb_next(node)) {
852+
tlink = rb_entry(node, struct tcon_link, tl_rbnode);
853+
tcon = tlink_tcon(tlink);
854+
if (IS_ERR(tcon))
855+
continue;
856+
tmp_list = kmalloc(sizeof(struct tcon_list), GFP_ATOMIC);
857+
if (tmp_list == NULL)
858+
break;
859+
tmp_list->tcon = tcon;
860+
/* Take a reference on tcon to prevent it from being freed */
861+
spin_lock(&tcon->tc_lock);
862+
++tcon->tc_count;
863+
trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count,
864+
netfs_trace_tcon_ref_get_close_defer_files);
865+
spin_unlock(&tcon->tc_lock);
866+
list_add_tail(&tmp_list->entry, &tcon_head);
867+
}
868+
spin_unlock(&cifs_sb->tlink_tree_lock);
869+
870+
list_for_each_entry_safe(tmp_list, q, &tcon_head, entry) {
871+
cifs_close_all_deferred_files(tmp_list->tcon);
872+
list_del(&tmp_list->entry);
873+
cifs_put_tcon(tmp_list->tcon, netfs_trace_tcon_ref_put_close_defer_files);
874+
kfree(tmp_list);
875+
}
876+
}
877+
836878
void cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon,
837879
struct dentry *dentry)
838880
{

fs/smb/client/trace.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
EM(netfs_trace_tcon_ref_get_cached_laundromat, "GET Ch-Lau") \
4848
EM(netfs_trace_tcon_ref_get_cached_lease_break, "GET Ch-Lea") \
4949
EM(netfs_trace_tcon_ref_get_cancelled_close, "GET Cn-Cls") \
50+
EM(netfs_trace_tcon_ref_get_close_defer_files, "GET Cl-Def") \
5051
EM(netfs_trace_tcon_ref_get_dfs_refer, "GET DfsRef") \
5152
EM(netfs_trace_tcon_ref_get_find, "GET Find ") \
5253
EM(netfs_trace_tcon_ref_get_find_sess_tcon, "GET FndSes") \
@@ -58,6 +59,7 @@
5859
EM(netfs_trace_tcon_ref_put_cancelled_close, "PUT Cn-Cls") \
5960
EM(netfs_trace_tcon_ref_put_cancelled_close_fid, "PUT Cn-Fid") \
6061
EM(netfs_trace_tcon_ref_put_cancelled_mid, "PUT Cn-Mid") \
62+
EM(netfs_trace_tcon_ref_put_close_defer_files, "PUT Cl-Def") \
6163
EM(netfs_trace_tcon_ref_put_mnt_ctx, "PUT MntCtx") \
6264
EM(netfs_trace_tcon_ref_put_dfs_refer, "PUT DfsRfr") \
6365
EM(netfs_trace_tcon_ref_put_reconnect_server, "PUT Reconn") \

0 commit comments

Comments
 (0)