added to cifs-2.6.git for-next pending additional review/testing
On Mon, Mar 17, 2025 at 2:39 PM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
>
> If @server->tcpStatus is set to CifsNeedReconnect after acquiring
> @ses->session_mutex in smb2_reconnect() or cifs_reconnect_tcon(), it
> means that a concurrent thread failed to negotiate, in which case the
> server is no longer responding to any SMB requests, so there is no
> point making the caller retry the IO by returning -EAGAIN.
>
> Fix this by returning -EHOSTDOWN to the callers on soft mounts.
>
> Cc: David Howells <dhowells@xxxxxxxxxx>
> Reported-by: Jay Shin <jaeshin@xxxxxxxxxx>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@xxxxxxxxxxxxx>
> ---
> fs/smb/client/cifssmb.c | 46 ++++++++++++--------
> fs/smb/client/smb2pdu.c | 96 ++++++++++++++++++-----------------------
> 2 files changed, 69 insertions(+), 73 deletions(-)
>
> diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> index d07682020c64..4fc9485c5d91 100644
> --- a/fs/smb/client/cifssmb.c
> +++ b/fs/smb/client/cifssmb.c
> @@ -114,19 +114,23 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
>
> mutex_lock(&ses->session_mutex);
> /*
> - * Recheck after acquire mutex. If another thread is negotiating
> - * and the server never sends an answer the socket will be closed
> - * and tcpStatus set to reconnect.
> + * Handle the case where a concurrent thread failed to negotiate or
> + * killed a channel.
> */
> spin_lock(&server->srv_lock);
> - if (server->tcpStatus == CifsNeedReconnect) {
> + switch (server->tcpStatus) {
> + case CifsExiting:
> spin_unlock(&server->srv_lock);
> mutex_unlock(&ses->session_mutex);
> -
> - if (tcon->retry)
> - goto again;
> - rc = -EHOSTDOWN;
> - goto out;
> + return -EHOSTDOWN;
> + case CifsNeedReconnect:
> + spin_unlock(&server->srv_lock);
> + mutex_unlock(&ses->session_mutex);
> + if (!tcon->retry)
> + return -EHOSTDOWN;
> + goto again;
> + default:
> + break;
> }
> spin_unlock(&server->srv_lock);
>
> @@ -152,16 +156,20 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
> spin_unlock(&ses->ses_lock);
>
> rc = cifs_negotiate_protocol(0, ses, server);
> - if (!rc) {
> - rc = cifs_setup_session(0, ses, server, ses->local_nls);
> - if ((rc == -EACCES) || (rc == -EHOSTDOWN) || (rc == -EKEYREVOKED)) {
> - /*
> - * Try alternate password for next reconnect if an alternate
> - * password is available.
> - */
> - if (ses->password2)
> - swap(ses->password2, ses->password);
> - }
> + if (rc) {
> + mutex_unlock(&ses->session_mutex);
> + if (!tcon->retry)
> + return -EHOSTDOWN;
> + goto again;
> + }
> + rc = cifs_setup_session(0, ses, server, ses->local_nls);
> + if ((rc == -EACCES) || (rc == -EHOSTDOWN) || (rc == -EKEYREVOKED)) {
> + /*
> + * Try alternate password for next reconnect if an alternate
> + * password is available.
> + */
> + if (ses->password2)
> + swap(ses->password2, ses->password);
> }
>
> /* do we need to reconnect tcon? */
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index ed7812247ebc..f9c521b3c65e 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -300,32 +300,23 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
>
> mutex_lock(&ses->session_mutex);
> /*
> - * if this is called by delayed work, and the channel has been disabled
> - * in parallel, the delayed work can continue to execute in parallel
> - * there's a chance that this channel may not exist anymore
> + * Handle the case where a concurrent thread failed to negotiate or
> + * killed a channel.
> */
> spin_lock(&server->srv_lock);
> - if (server->tcpStatus == CifsExiting) {
> + switch (server->tcpStatus) {
> + case CifsExiting:
> spin_unlock(&server->srv_lock);
> mutex_unlock(&ses->session_mutex);
> - rc = -EHOSTDOWN;
> - goto out;
> - }
> -
> - /*
> - * Recheck after acquire mutex. If another thread is negotiating
> - * and the server never sends an answer the socket will be closed
> - * and tcpStatus set to reconnect.
> - */
> - if (server->tcpStatus == CifsNeedReconnect) {
> + return -EHOSTDOWN;
> + case CifsNeedReconnect:
> spin_unlock(&server->srv_lock);
> mutex_unlock(&ses->session_mutex);
> -
> - if (tcon->retry)
> - goto again;
> -
> - rc = -EHOSTDOWN;
> - goto out;
> + if (!tcon->retry)
> + return -EHOSTDOWN;
> + goto again;
> + default:
> + break;
> }
> spin_unlock(&server->srv_lock);
>
> @@ -350,43 +341,41 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> spin_unlock(&ses->ses_lock);
>
> rc = cifs_negotiate_protocol(0, ses, server);
> - if (!rc) {
> - /*
> - * if server stopped supporting multichannel
> - * and the first channel reconnected, disable all the others.
> - */
> - if (ses->chan_count > 1 &&
> - !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
> - rc = cifs_chan_skip_or_disable(ses, server,
> - from_reconnect);
> - if (rc) {
> - mutex_unlock(&ses->session_mutex);
> - goto out;
> - }
> - }
> -
> - rc = cifs_setup_session(0, ses, server, ses->local_nls);
> - if ((rc == -EACCES) || (rc == -EKEYEXPIRED) || (rc == -EKEYREVOKED)) {
> - /*
> - * Try alternate password for next reconnect (key rotation
> - * could be enabled on the server e.g.) if an alternate
> - * password is available and the current password is expired,
> - * but do not swap on non pwd related errors like host down
> - */
> - if (ses->password2)
> - swap(ses->password2, ses->password);
> - }
> -
> - if ((rc == -EACCES) && !tcon->retry) {
> - mutex_unlock(&ses->session_mutex);
> - rc = -EHOSTDOWN;
> - goto failed;
> - } else if (rc) {
> + if (rc) {
> + mutex_unlock(&ses->session_mutex);
> + if (!tcon->retry)
> + return -EHOSTDOWN;
> + goto again;
> + }
> + /*
> + * if server stopped supporting multichannel
> + * and the first channel reconnected, disable all the others.
> + */
> + if (ses->chan_count > 1 &&
> + !(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
> + rc = cifs_chan_skip_or_disable(ses, server,
> + from_reconnect);
> + if (rc) {
> mutex_unlock(&ses->session_mutex);
> goto out;
> }
> - } else {
> + }
> +
> + rc = cifs_setup_session(0, ses, server, ses->local_nls);
> + if ((rc == -EACCES) || (rc == -EKEYEXPIRED) || (rc == -EKEYREVOKED)) {
> + /*
> + * Try alternate password for next reconnect (key rotation
> + * could be enabled on the server e.g.) if an alternate
> + * password is available and the current password is expired,
> + * but do not swap on non pwd related errors like host down
> + */
> + if (ses->password2)
> + swap(ses->password2, ses->password);
> + }
> + if (rc) {
> mutex_unlock(&ses->session_mutex);
> + if (rc == -EACCES && !tcon->retry)
> + return -EHOSTDOWN;
> goto out;
> }
>
> @@ -490,7 +479,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> case SMB2_IOCTL:
> rc = -EAGAIN;
> }
> -failed:
> return rc;
> }
>
> --
> 2.48.1
>
--
Thanks,
Steve
