Skip to content

Commit cc391b6

Browse files
vwaxSteve French
authored and
Steve French
committed
cifs: fix potential deadlock in direct reclaim
The srv_mutex is used during writeback so cifs should ensure that allocations done when that mutex is held are done with GFP_NOFS, to avoid having direct reclaim ending up waiting for the same mutex and causing a deadlock. This is detected by lockdep with the splat below: ====================================================== WARNING: possible circular locking dependency detected 5.18.0 #70 Not tainted ------------------------------------------------------ kswapd0/49 is trying to acquire lock: ffff8880195782e0 (&tcp_ses->srv_mutex){+.+.}-{3:3}, at: compound_send_recv but task is already holding lock: ffffffffa98e66c0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (fs_reclaim){+.+.}-{0:0}: fs_reclaim_acquire kmem_cache_alloc_trace __request_module crypto_alg_mod_lookup crypto_alloc_tfm_node crypto_alloc_shash cifs_alloc_hash smb311_crypto_shash_allocate smb311_update_preauth_hash compound_send_recv cifs_send_recv SMB2_negotiate smb2_negotiate cifs_negotiate_protocol cifs_get_smb_ses cifs_mount cifs_smb3_do_mount smb3_get_tree vfs_get_tree path_mount __x64_sys_mount do_syscall_64 entry_SYSCALL_64_after_hwframe -> #0 (&tcp_ses->srv_mutex){+.+.}-{3:3}: __lock_acquire lock_acquire __mutex_lock mutex_lock_nested compound_send_recv cifs_send_recv SMB2_write smb2_sync_write cifs_write cifs_writepage_locked cifs_writepage shrink_page_list shrink_lruvec shrink_node balance_pgdat kswapd kthread ret_from_fork other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(fs_reclaim); lock(&tcp_ses->srv_mutex); lock(fs_reclaim); lock(&tcp_ses->srv_mutex); *** DEADLOCK *** 1 lock held by kswapd0/49: #0: ffffffffa98e66c0 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat stack backtrace: CPU: 2 PID: 49 Comm: kswapd0 Not tainted 5.18.0 #70 Call Trace: <TASK> dump_stack_lvl dump_stack print_circular_bug.cold check_noncircular __lock_acquire lock_acquire __mutex_lock mutex_lock_nested compound_send_recv cifs_send_recv SMB2_write smb2_sync_write cifs_write cifs_writepage_locked cifs_writepage shrink_page_list shrink_lruvec shrink_node balance_pgdat kswapd kthread ret_from_fork </TASK> Fix this by using the memalloc_nofs_save/restore APIs around the places where the srv_mutex is held. Do this in a wrapper function for the lock/unlock of the srv_mutex, and rename the srv_mutex to avoid missing call sites in the conversion. Note that there is another lockdep warning involving internal crypto locks, which was masked by this problem and is visible after this fix, see the discussion in this thread: https://lore.kernel.org/all/[email protected]/ Link: https://lore.kernel.org/r/CANT5p=rqcYfYMVHirqvdnnca4Mo+JQSw5Qu12v=kPfpk5yhhmg@mail.gmail.com/ Reported-by: Shyam Prasad N <[email protected]> Suggested-by: Lars Persson <[email protected]> Reviewed-by: Ronnie Sahlberg <[email protected]> Reviewed-by: Enzo Matsumiya <[email protected]> Signed-off-by: Vincent Whitchurch <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent f66f8b9 commit cc391b6

10 files changed

+71
-53
lines changed

fs/cifs/cifs_swn.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
465465
int ret = 0;
466466

467467
/* Store the reconnect address */
468-
mutex_lock(&tcon->ses->server->srv_mutex);
468+
cifs_server_lock(tcon->ses->server);
469469
if (cifs_sockaddr_equal(&tcon->ses->server->dstaddr, addr))
470470
goto unlock;
471471

@@ -501,7 +501,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
501501
cifs_signal_cifsd_for_reconnect(tcon->ses->server, false);
502502

503503
unlock:
504-
mutex_unlock(&tcon->ses->server->srv_mutex);
504+
cifs_server_unlock(tcon->ses->server);
505505

506506
return ret;
507507
}

fs/cifs/cifsencrypt.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,9 @@ int cifs_verify_signature(struct smb_rqst *rqst,
236236
cpu_to_le32(expected_sequence_number);
237237
cifs_pdu->Signature.Sequence.Reserved = 0;
238238

239-
mutex_lock(&server->srv_mutex);
239+
cifs_server_lock(server);
240240
rc = cifs_calc_signature(rqst, server, what_we_think_sig_should_be);
241-
mutex_unlock(&server->srv_mutex);
241+
cifs_server_unlock(server);
242242

243243
if (rc)
244244
return rc;
@@ -626,7 +626,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
626626

627627
memcpy(ses->auth_key.response + baselen, tiblob, tilen);
628628

629-
mutex_lock(&ses->server->srv_mutex);
629+
cifs_server_lock(ses->server);
630630

631631
rc = cifs_alloc_hash("hmac(md5)",
632632
&ses->server->secmech.hmacmd5,
@@ -678,7 +678,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
678678
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
679679

680680
unlock:
681-
mutex_unlock(&ses->server->srv_mutex);
681+
cifs_server_unlock(ses->server);
682682
setup_ntlmv2_rsp_ret:
683683
kfree(tiblob);
684684

fs/cifs/cifsglob.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <linux/mempool.h>
1717
#include <linux/workqueue.h>
1818
#include <linux/utsname.h>
19+
#include <linux/sched/mm.h>
1920
#include <linux/netfs.h>
2021
#include "cifs_fs_sb.h"
2122
#include "cifsacl.h"
@@ -628,7 +629,8 @@ struct TCP_Server_Info {
628629
unsigned int in_flight; /* number of requests on the wire to server */
629630
unsigned int max_in_flight; /* max number of requests that were on wire */
630631
spinlock_t req_lock; /* protect the two values above */
631-
struct mutex srv_mutex;
632+
struct mutex _srv_mutex;
633+
unsigned int nofs_flag;
632634
struct task_struct *tsk;
633635
char server_GUID[16];
634636
__u16 sec_mode;
@@ -743,6 +745,22 @@ struct TCP_Server_Info {
743745
#endif
744746
};
745747

748+
static inline void cifs_server_lock(struct TCP_Server_Info *server)
749+
{
750+
unsigned int nofs_flag = memalloc_nofs_save();
751+
752+
mutex_lock(&server->_srv_mutex);
753+
server->nofs_flag = nofs_flag;
754+
}
755+
756+
static inline void cifs_server_unlock(struct TCP_Server_Info *server)
757+
{
758+
unsigned int nofs_flag = server->nofs_flag;
759+
760+
mutex_unlock(&server->_srv_mutex);
761+
memalloc_nofs_restore(nofs_flag);
762+
}
763+
746764
struct cifs_credits {
747765
unsigned int value;
748766
unsigned int instance;

fs/cifs/connect.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ static void cifs_resolve_server(struct work_struct *work)
148148
struct TCP_Server_Info *server = container_of(work,
149149
struct TCP_Server_Info, resolve.work);
150150

151-
mutex_lock(&server->srv_mutex);
151+
cifs_server_lock(server);
152152

153153
/*
154154
* Resolve the hostname again to make sure that IP address is up-to-date.
@@ -159,7 +159,7 @@ static void cifs_resolve_server(struct work_struct *work)
159159
__func__, rc);
160160
}
161161

162-
mutex_unlock(&server->srv_mutex);
162+
cifs_server_unlock(server);
163163
}
164164

165165
/*
@@ -267,7 +267,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
267267

268268
/* do not want to be sending data on a socket we are freeing */
269269
cifs_dbg(FYI, "%s: tearing down socket\n", __func__);
270-
mutex_lock(&server->srv_mutex);
270+
cifs_server_lock(server);
271271
if (server->ssocket) {
272272
cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n", server->ssocket->state,
273273
server->ssocket->flags);
@@ -296,7 +296,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
296296
mid->mid_flags |= MID_DELETED;
297297
}
298298
spin_unlock(&GlobalMid_Lock);
299-
mutex_unlock(&server->srv_mutex);
299+
cifs_server_unlock(server);
300300

301301
cifs_dbg(FYI, "%s: issuing mid callbacks\n", __func__);
302302
list_for_each_entry_safe(mid, nmid, &retry_list, qhead) {
@@ -306,9 +306,9 @@ cifs_abort_connection(struct TCP_Server_Info *server)
306306
}
307307

308308
if (cifs_rdma_enabled(server)) {
309-
mutex_lock(&server->srv_mutex);
309+
cifs_server_lock(server);
310310
smbd_destroy(server);
311-
mutex_unlock(&server->srv_mutex);
311+
cifs_server_unlock(server);
312312
}
313313
}
314314

@@ -359,7 +359,7 @@ static int __cifs_reconnect(struct TCP_Server_Info *server,
359359

360360
do {
361361
try_to_freeze();
362-
mutex_lock(&server->srv_mutex);
362+
cifs_server_lock(server);
363363

364364
if (!cifs_swn_set_server_dstaddr(server)) {
365365
/* resolve the hostname again to make sure that IP address is up-to-date */
@@ -372,7 +372,7 @@ static int __cifs_reconnect(struct TCP_Server_Info *server,
372372
else
373373
rc = generic_ip_connect(server);
374374
if (rc) {
375-
mutex_unlock(&server->srv_mutex);
375+
cifs_server_unlock(server);
376376
cifs_dbg(FYI, "%s: reconnect error %d\n", __func__, rc);
377377
msleep(3000);
378378
} else {
@@ -383,7 +383,7 @@ static int __cifs_reconnect(struct TCP_Server_Info *server,
383383
server->tcpStatus = CifsNeedNegotiate;
384384
spin_unlock(&cifs_tcp_ses_lock);
385385
cifs_swn_reset_server_dstaddr(server);
386-
mutex_unlock(&server->srv_mutex);
386+
cifs_server_unlock(server);
387387
mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
388388
}
389389
} while (server->tcpStatus == CifsNeedReconnect);
@@ -488,12 +488,12 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server)
488488

489489
do {
490490
try_to_freeze();
491-
mutex_lock(&server->srv_mutex);
491+
cifs_server_lock(server);
492492

493493
rc = reconnect_target_unlocked(server, &tl, &target_hint);
494494
if (rc) {
495495
/* Failed to reconnect socket */
496-
mutex_unlock(&server->srv_mutex);
496+
cifs_server_unlock(server);
497497
cifs_dbg(FYI, "%s: reconnect error %d\n", __func__, rc);
498498
msleep(3000);
499499
continue;
@@ -510,7 +510,7 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server)
510510
server->tcpStatus = CifsNeedNegotiate;
511511
spin_unlock(&cifs_tcp_ses_lock);
512512
cifs_swn_reset_server_dstaddr(server);
513-
mutex_unlock(&server->srv_mutex);
513+
cifs_server_unlock(server);
514514
mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
515515
} while (server->tcpStatus == CifsNeedReconnect);
516516

@@ -1565,7 +1565,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
15651565
init_waitqueue_head(&tcp_ses->response_q);
15661566
init_waitqueue_head(&tcp_ses->request_q);
15671567
INIT_LIST_HEAD(&tcp_ses->pending_mid_q);
1568-
mutex_init(&tcp_ses->srv_mutex);
1568+
mutex_init(&tcp_ses->_srv_mutex);
15691569
memcpy(tcp_ses->workstation_RFC1001_name,
15701570
ctx->source_rfc1001_name, RFC1001_NAME_LEN_WITH_NULL);
15711571
memcpy(tcp_ses->server_RFC1001_name,

fs/cifs/dfs_cache.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1327,9 +1327,9 @@ static bool target_share_equal(struct TCP_Server_Info *server, const char *s1, c
13271327
cifs_dbg(VFS, "%s: failed to convert address \'%s\'. skip address matching.\n",
13281328
__func__, ip);
13291329
} else {
1330-
mutex_lock(&server->srv_mutex);
1330+
cifs_server_lock(server);
13311331
match = cifs_match_ipaddr((struct sockaddr *)&server->dstaddr, &sa);
1332-
mutex_unlock(&server->srv_mutex);
1332+
cifs_server_unlock(server);
13331333
}
13341334

13351335
kfree(ip);

fs/cifs/sess.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,14 +1120,14 @@ sess_establish_session(struct sess_data *sess_data)
11201120
struct cifs_ses *ses = sess_data->ses;
11211121
struct TCP_Server_Info *server = sess_data->server;
11221122

1123-
mutex_lock(&server->srv_mutex);
1123+
cifs_server_lock(server);
11241124
if (!server->session_estab) {
11251125
if (server->sign) {
11261126
server->session_key.response =
11271127
kmemdup(ses->auth_key.response,
11281128
ses->auth_key.len, GFP_KERNEL);
11291129
if (!server->session_key.response) {
1130-
mutex_unlock(&server->srv_mutex);
1130+
cifs_server_unlock(server);
11311131
return -ENOMEM;
11321132
}
11331133
server->session_key.len =
@@ -1136,7 +1136,7 @@ sess_establish_session(struct sess_data *sess_data)
11361136
server->sequence_number = 0x2;
11371137
server->session_estab = true;
11381138
}
1139-
mutex_unlock(&server->srv_mutex);
1139+
cifs_server_unlock(server);
11401140

11411141
cifs_dbg(FYI, "CIFS session established successfully\n");
11421142
return 0;

fs/cifs/smb1ops.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_rqst *rqst,
3838
in_buf->WordCount = 0;
3939
put_bcc(0, in_buf);
4040

41-
mutex_lock(&server->srv_mutex);
41+
cifs_server_lock(server);
4242
rc = cifs_sign_smb(in_buf, server, &mid->sequence_number);
4343
if (rc) {
44-
mutex_unlock(&server->srv_mutex);
44+
cifs_server_unlock(server);
4545
return rc;
4646
}
4747

@@ -55,7 +55,7 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_rqst *rqst,
5555
if (rc < 0)
5656
server->sequence_number--;
5757

58-
mutex_unlock(&server->srv_mutex);
58+
cifs_server_unlock(server);
5959

6060
cifs_dbg(FYI, "issued NT_CANCEL for mid %u, rc = %d\n",
6161
get_mid(in_buf), rc);

fs/cifs/smb2pdu.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,21 +1369,21 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data)
13691369
struct cifs_ses *ses = sess_data->ses;
13701370
struct TCP_Server_Info *server = sess_data->server;
13711371

1372-
mutex_lock(&server->srv_mutex);
1372+
cifs_server_lock(server);
13731373
if (server->ops->generate_signingkey) {
13741374
rc = server->ops->generate_signingkey(ses, server);
13751375
if (rc) {
13761376
cifs_dbg(FYI,
13771377
"SMB3 session key generation failed\n");
1378-
mutex_unlock(&server->srv_mutex);
1378+
cifs_server_unlock(server);
13791379
return rc;
13801380
}
13811381
}
13821382
if (!server->session_estab) {
13831383
server->sequence_number = 0x2;
13841384
server->session_estab = true;
13851385
}
1386-
mutex_unlock(&server->srv_mutex);
1386+
cifs_server_unlock(server);
13871387

13881388
cifs_dbg(FYI, "SMB2/3 session established successfully\n");
13891389
return rc;

fs/cifs/smbdirect.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,9 +1382,9 @@ void smbd_destroy(struct TCP_Server_Info *server)
13821382
log_rdma_event(INFO, "freeing mr list\n");
13831383
wake_up_interruptible_all(&info->wait_mr);
13841384
while (atomic_read(&info->mr_used_count)) {
1385-
mutex_unlock(&server->srv_mutex);
1385+
cifs_server_unlock(server);
13861386
msleep(1000);
1387-
mutex_lock(&server->srv_mutex);
1387+
cifs_server_lock(server);
13881388
}
13891389
destroy_mr_list(info);
13901390

0 commit comments

Comments
 (0)