linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Metzmacher <metze@samba.org>
To: linux-cifs@vger.kernel.org
Cc: Stefan Metzmacher <metze@samba.org>
Subject: [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state
Date: Mon, 24 Feb 2020 14:15:10 +0100	[thread overview]
Message-ID: <20200224131510.20608-14-metze@samba.org> (raw)
In-Reply-To: <20200224131510.20608-1-metze@samba.org>

RHBZ: 1579050

If a session setup gets NT_STATUS_LOGON_FAILURE of
NT_STATUS_ACCOUNT_LOCKED_OUT, we're not able to make any further
progress with the credentials the kernel knowns about.

Instead of retrying for each new request, we should mark
the session with CifsInvalidCredentials (for know we only do that
for soft mounts). We consistently return -EKEYREVOKED for such sessions,
I guess that's better than -EHOSTDOWN.

A future addition would be an upcall to get new credentials from
userspace, or a way to use a magic file per session under /proc/fs/cifs/
to provide new credentials.

But for now we want to avoid that we lock out the users account,
by retrying every 2 seconds.

Note backports also need the other 12 patches before.

Fixes: b0dd940e582b6a6 ("cifs: fail i/o on soft mounts if sessionsetup errors out")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Stable <stable@vger.kernel.org>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifs_debug.c |   2 +
 fs/cifs/cifsfs.c     |   1 +
 fs/cifs/cifsglob.h   |   4 +-
 fs/cifs/connect.c    | 145 ++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 148 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 276e4b5ea8e0..c03a445e95bc 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -392,6 +392,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 
 			/* dump session id helpful for use with network trace */
 			seq_printf(m, " SessionId: 0x%llx", ses->Suid);
+			if (ses->status == CifsInvalidCredentials)
+				seq_printf(m, " InvalidCredentials!");
 			if (ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA)
 				seq_puts(m, " encrypted");
 			if (ses->sign)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 46ebaf3f0824..9e2e5fa2cdff 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -650,6 +650,7 @@ static void cifs_umount_begin(struct super_block *sb)
 	/* cancel_notify_requests(tcon); */
 	if (tcon->ses && tcon->ses->server) {
 		cifs_dbg(FYI, "wake up tasks now - umount begin not complete\n");
+		wake_up_all(&tcon->ses->server->demultiplex_q);
 		wake_up_all(&tcon->ses->server->request_q);
 		wake_up_all(&tcon->ses->server->response_q);
 		msleep(1); /* yield */
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8393ed7ebf96..4494b35fa5c5 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -114,7 +114,8 @@ enum statusEnum {
 	CifsGood,
 	CifsExiting,
 	CifsNeedReconnect,
-	CifsNeedNegotiate
+	CifsNeedNegotiate,
+	CifsInvalidCredentials,
 };
 
 enum securityEnum {
@@ -687,6 +688,7 @@ struct TCP_Server_Info {
 #endif
 	wait_queue_head_t response_q;
 	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
+	wait_queue_head_t demultiplex_q;
 	struct list_head pending_mid_q;
 	bool noblocksnd;		/* use blocking sendmsg */
 	bool noautotune;		/* do not autotune send buf sizes */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7f4be85b7cc9..39c0a5154eb9 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -480,6 +480,65 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
 }
 #endif
 
+static bool
+server_should_poll_for_responses(struct TCP_Server_Info *server)
+{
+	switch (server->tcpStatus) {
+	case CifsNew:
+		return false;
+	case CifsExiting:
+		return false;
+	case CifsGood:
+		return true;
+	case CifsNeedReconnect:
+		return false;
+	case CifsNeedNegotiate:
+		return true;
+	case CifsInvalidCredentials:
+		/*
+		 * Only a session should have this state
+		 */
+		WARN(true, "BUG! server->tcpStatus=CifsInvalidCredentials\n");
+		return false;
+	}
+
+	WARN(true, "Unhandled server->tcpStatus=%u\n", server->tcpStatus);
+	return false;
+}
+
+static bool
+server_should_reconnect(struct TCP_Server_Info *server)
+{
+	struct cifs_ses *ses;
+	size_t num_valid = 0;
+	size_t num_invalid = 0;
+
+	/*
+	 * If we are in disconnected state, waiting for a reconnect,
+	 * we should only try to reconnect if there's a chance
+	 * of success, that's not the case of all sessions
+	 * are have invalid credentials.
+	 */
+	if (server->tcpStatus != CifsNeedReconnect)
+		return false;
+
+	spin_lock(&cifs_tcp_ses_lock);
+	list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+		if (ses->status != CifsInvalidCredentials) {
+			num_valid++;
+		} else {
+			num_invalid++;
+		}
+	}
+	spin_unlock(&cifs_tcp_ses_lock);
+
+	if (num_invalid == 0 || num_valid != 0) {
+		return true;
+	}
+
+	return false;
+}
+
 /*
  * cifs tcp session reconnection
  *
@@ -617,6 +676,17 @@ cifs_reconnect(struct TCP_Server_Info *server)
 		try_to_freeze();
 
 		mutex_lock(&server->srv_mutex);
+
+		if (!server_should_reconnect(server)) {
+			cifs_dbg(VFS,
+				 "Server %s was disconnected. "
+				 "We should NOT reconnect...\n",
+				 server->hostname);
+			rc = -ECONNABORTED;
+			mutex_unlock(&server->srv_mutex);
+			break;
+		}
+
 		/*
 		 * Set up next DFS target server (if any) for reconnect. If DFS
 		 * feature is disabled, then we will retry last server we
@@ -669,8 +739,17 @@ cifs_reconnect(struct TCP_Server_Info *server)
 
 	put_tcp_super(sb);
 #endif
-	if (server->tcpStatus == CifsNeedNegotiate)
-		mod_delayed_work(cifsiod_wq, &server->echo, 0);
+	if (server->tcpStatus == CifsNeedNegotiate) {
+		/* unfreeze the demultiplex thread */
+		wake_up_all(&server->demultiplex_q);
+		/*
+		 * schedule a immediate reconnect of sessions
+		 * and tcons.
+		 */
+		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+		/* reset the echo timer into the future. */
+		mod_delayed_work(cifsiod_wq, &server->echo, server->echo_interval);
+	}
 
 	wake_up(&server->response_q);
 	return rc;
@@ -872,6 +951,19 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)
 	for (total_read = 0; msg_data_left(smb_msg); total_read += length) {
 		try_to_freeze();
 
+		if (server_should_reconnect(server)) {
+			cifs_dbg(VFS, "Server %s was disconnected. Reconnecting...\n",
+				 server->hostname);
+			cifs_reconnect(server);
+			wake_up(&server->response_q);
+			return -ECONNABORTED;
+		}
+
+		if (!server_should_poll_for_responses(server)) {
+			cifs_dbg(FYI, "Only invalid credential sessions\n");
+			return -ECONNABORTED;
+		}
+
 		/* reconnect if no credits and no requests in flight */
 		if (zero_credits(server)) {
 			cifs_reconnect(server);
@@ -1222,6 +1314,22 @@ smb2_add_credits_from_hdr(char *buffer, struct TCP_Server_Info *server)
 	}
 }
 
+static bool cifs_demultiplex_thread_should_block(struct TCP_Server_Info *server)
+{
+	if (server->tcpStatus == CifsExiting) {
+		return false;
+	}
+
+	if (server_should_reconnect(server)) {
+		return false;
+	}
+
+	if (server_should_poll_for_responses(server)) {
+		return false;
+	}
+
+	return true;
+}
 
 static int
 cifs_demultiplex_thread(void *p)
@@ -1248,6 +1356,11 @@ cifs_demultiplex_thread(void *p)
 		if (try_to_freeze())
 			continue;
 
+		wait_event_freezable(server->demultiplex_q,
+				!cifs_demultiplex_thread_should_block(server));
+		if (server->tcpStatus == CifsExiting)
+			break;
+
 		if (!allocate_buffers(server))
 			continue;
 
@@ -2880,6 +2993,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 	tcp_ses->credits = 1;
 	init_waitqueue_head(&tcp_ses->response_q);
 	init_waitqueue_head(&tcp_ses->request_q);
+	init_waitqueue_head(&tcp_ses->demultiplex_q);
 	INIT_LIST_HEAD(&tcp_ses->pending_mid_q);
 	mutex_init(&tcp_ses->srv_mutex);
 	memcpy(tcp_ses->workstation_RFC1001_name,
@@ -2967,6 +3081,13 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 
 	cifs_fscache_get_client_cookie(tcp_ses);
 
+	/* unfreeze the demultiplex thread */
+	wake_up_all(&tcp_ses->demultiplex_q);
+	/*
+	 * we let the caller use cifs_connect_session_locked()
+	 * (directly or via cifs_get_smb_ses() and don't
+	 * schedule the reconnect timer.
+	 */
 	/* queue echo request delayed work */
 	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
 
@@ -5454,6 +5575,10 @@ cifs_connect_session_locked(const unsigned int xid,
 {
 	int rc;
 
+	if (ses->status == CifsInvalidCredentials) {
+		return -EKEYREVOKED;
+	}
+
 	if (ses->server->tcpStatus == CifsNeedReconnect) {
 		return -EHOSTDOWN;
 	}
@@ -5463,7 +5588,12 @@ cifs_connect_session_locked(const unsigned int xid,
 		cifs_dbg(FYI, "Session needs reconnect\n");
 		rc = cifs_setup_session(xid, ses, nls_info);
 		if ((rc == -EACCES) && !retry) {
-			return -EHOSTDOWN;
+			cifs_dbg(VFS,
+				 "SessionSetup to Server %s failed, "
+				 "marking as CifsInvalidCredentials/EKEYREVOKED\n",
+				 ses->server->hostname);
+			ses->status = CifsInvalidCredentials;
+			return -EKEYREVOKED;
 		}
 	}
 
@@ -5505,6 +5635,10 @@ cifs_tcon_reconnect(struct cifs_tcon *tcon,
 
 	retries = server->nr_targets;
 
+	if (ses->status == CifsInvalidCredentials) {
+		return -EKEYREVOKED;
+	}
+
 	/*
 	 * Give demultiplex thread up to 10 seconds to each target available for
 	 * reconnect -- should be greater than cifs socket timeout which is 7
@@ -5515,6 +5649,7 @@ cifs_tcon_reconnect(struct cifs_tcon *tcon,
 			return -EAGAIN;
 		}
 
+		wake_up_all(&server->demultiplex_q);
 		rc = wait_event_interruptible_timeout(server->response_q,
 						      (server->tcpStatus != CifsNeedReconnect),
 						      10 * HZ);
@@ -5543,6 +5678,10 @@ cifs_tcon_reconnect(struct cifs_tcon *tcon,
 		retries = server->nr_targets;
 	}
 
+	if (ses->status == CifsInvalidCredentials) {
+		return -EKEYREVOKED;
+	}
+
 	if (!ses->need_reconnect && !tcon->need_reconnect)
 		return 0;
 
-- 
2.17.1


  parent reply	other threads:[~2020-02-24 13:22 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
2020-02-24 13:14 ` [PATCH v1 01/13] cifs: call wake_up(&server->response_q) inside of cifs_reconnect() Stefan Metzmacher
2020-02-24 13:14 ` [PATCH v1 02/13] cifs: use mod_delayed_work() for &server->reconnect if already queued Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon() Stefan Metzmacher
2020-02-24 18:12   ` Aurélien Aptel
2020-02-24 20:39     ` Steve French
2020-02-24 21:36       ` Pavel Shilovsky
2020-02-24 13:15 ` [PATCH v1 04/13] cifs: split out a cifs_connect_session_locked() helper function Stefan Metzmacher
2020-03-06 17:05   ` Aurélien Aptel
2020-02-24 13:15 ` [PATCH v1 05/13] cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect() Stefan Metzmacher
2020-03-06 16:46   ` Aurélien Aptel
2020-02-24 13:15 ` [PATCH v1 06/13] cifs: abstract cifs_tcon_reconnect() out of smb2_reconnect() Stefan Metzmacher
2020-03-27 17:31   ` Steve French
2020-02-24 13:15 ` [PATCH v1 07/13] cifs: cifs_reconnect_tcon() make use of the generic cifs_tcon_reconnect() function Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 08/13] cifs: make cifs_tree_connect() static Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 09/13] cifs: turn smb2_reconnect_server() into a generic cifs_reconnect_server() Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 10/13] cifs: move cifs_reconnect_tcons() to fs/cifs/connect.c and make it static Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 11/13] cifs: use reconnect timer also for SMB1 Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 12/13] cifs: map STATUS_ACCOUNT_LOCKED_OUT to -EACCES Stefan Metzmacher
2020-02-24 13:15 ` Stefan Metzmacher [this message]
2020-02-24 18:09   ` [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state Aurélien Aptel
2020-02-24 18:38 ` [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Aurélien Aptel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200224131510.20608-14-metze@samba.org \
    --to=metze@samba.org \
    --cc=linux-cifs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).