From: "Pali Rohár" <pali@kernel.org>
To: Steve French <sfrench@samba.org>,
Paulo Alcantara <pc@manguebit.com>,
Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 5/6] cifs: Fix negotiate retry functionality
Date: Sun, 22 Dec 2024 17:30:49 +0100 [thread overview]
Message-ID: <20241222163050.24359-6-pali@kernel.org> (raw)
In-Reply-To: <20241222163050.24359-1-pali@kernel.org>
SMB negotiate retry functionality in cifs_negotiate() is currently broken
and does not work when doing socket reconnect. Caller of this function,
which is cifs_negotiate_protocol() requires that tcpStatus after successful
execution of negotiate callback stay in CifsInNegotiate. But if the
CIFSSMBNegotiate() called from cifs_negotiate() fails due to connection
issues then tcpStatus is changed as so repeated CIFSSMBNegotiate() call
does not help.
Fix this problem by moving retrying code from negotiate callback (which is
either cifs_negotiate() or smb2_negotiate()) to cifs_negotiate_protocol()
which is caller of those callbacks. This allows to properly handle and
implement correct transistions between tcpStatus states as function
cifs_negotiate_protocol() already handles it.
With this change, cifs_negotiate_protocol() now handles also -EAGAIN error
set by the RFC1002_NEGATIVE_SESSION_RESPONSE processing after reconnecting
with NetBIOS session.
Signed-off-by: Pali Rohár <pali@kernel.org>
---
fs/smb/client/connect.c | 10 ++++++++++
fs/smb/client/smb1ops.c | 7 -------
fs/smb/client/smb2ops.c | 3 ---
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 9a76cdf71086..fdeeb0b9000d 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -4206,11 +4206,13 @@ int
cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
struct TCP_Server_Info *server)
{
+ bool in_retry = false;
int rc = 0;
if (!server->ops->need_neg || !server->ops->negotiate)
return -ENOSYS;
+retry:
/* only send once per connect */
spin_lock(&server->srv_lock);
if (server->tcpStatus != CifsGood &&
@@ -4230,6 +4232,14 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
spin_unlock(&server->srv_lock);
rc = server->ops->negotiate(xid, ses, server);
+ if (rc == -EAGAIN) {
+ /* Allow one retry attempt */
+ if (!in_retry) {
+ in_retry = true;
+ goto retry;
+ }
+ rc = -EHOSTDOWN;
+ }
if (rc == 0) {
spin_lock(&server->srv_lock);
if (server->tcpStatus == CifsInNegotiate)
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 01a7d6b23c7e..0533aca770fc 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -426,13 +426,6 @@ cifs_negotiate(const unsigned int xid,
{
int rc;
rc = CIFSSMBNegotiate(xid, ses, server);
- if (rc == -EAGAIN) {
- /* retry only once on 1st time connection */
- set_credits(server, 1);
- rc = CIFSSMBNegotiate(xid, ses, server);
- if (rc == -EAGAIN)
- rc = -EHOSTDOWN;
- }
return rc;
}
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 50adb5cdfce9..119ecb6641d1 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -464,9 +464,6 @@ smb2_negotiate(const unsigned int xid,
server->CurrentMid = 0;
spin_unlock(&server->mid_lock);
rc = SMB2_negotiate(xid, ses, server);
- /* BB we probably don't need to retry with modern servers */
- if (rc == -EAGAIN)
- rc = -EHOSTDOWN;
return rc;
}
--
2.20.1
next prev parent reply other threads:[~2024-12-22 16:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-22 16:30 [PATCH 0/6] cifs: Fix connections over NetBIOS session Pali Rohár
2024-12-22 16:30 ` [PATCH 1/6] cifs: Allow to disable or force initialization of " Pali Rohár
2024-12-22 16:30 ` [PATCH 2/6] cifs: Fix establishing NetBIOS session for SMB2+ connection Pali Rohár
2024-12-22 16:30 ` [PATCH 3/6] cifs: Improve establishing SMB connection with NetBIOS session Pali Rohár
2024-12-22 16:30 ` [PATCH 4/6] cifs: Improve handling of NetBIOS packets Pali Rohár
2024-12-22 16:30 ` Pali Rohár [this message]
2024-12-22 16:30 ` [PATCH 6/6] cifs: Set default Netbios RFC1001 server name to hostname in UNC Pali Rohár
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=20241222163050.24359-6-pali@kernel.org \
--to=pali@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pc@manguebit.com \
--cc=ronniesahlberg@gmail.com \
--cc=sfrench@samba.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