linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.6 10/21] smb: Work around Clang __bdos() type confusion
       [not found] <20240202184015.540966-1-sashal@kernel.org>
@ 2024-02-02 18:39 ` Sasha Levin
  2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 11/21] cifs: cifs_pick_channel should try selecting active channels Sasha Levin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-02-02 18:39 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Kees Cook, Nathan Chancellor, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, linux-cifs, llvm,
	Steve French, Sasha Levin, samba-technical

From: Kees Cook <keescook@chromium.org>

[ Upstream commit 8deb05c84b63b4fdb8549e08942867a68924a5b8 ]

Recent versions of Clang gets confused about the possible size of the
"user" allocation, and CONFIG_FORTIFY_SOURCE ends up emitting a
warning[1]:

repro.c:126:4: warning: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
  126 |                         __write_overflow_field(p_size_field, size);
      |                         ^

for this memset():

        int len;
        __le16 *user;
	...
        len = ses->user_name ? strlen(ses->user_name) : 0;
        user = kmalloc(2 + (len * 2), GFP_KERNEL);
	...
	if (len) {
		...
	} else {
		memset(user, '\0', 2);
	}

While Clang works on this bug[2], switch to using a direct assignment,
which avoids memset() entirely which both simplifies the code and silences
the false positive warning. (Making "len" size_t also silences the
warning, but the direct assignment seems better.)

Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://github.com/ClangBuiltLinux/linux/issues/1966 [1]
Link: https://github.com/llvm/llvm-project/issues/77813 [2]
Cc: Steve French <sfrench@samba.org>
Cc: Paulo Alcantara <pc@manguebit.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: llvm@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/smb/client/cifsencrypt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/cifsencrypt.c b/fs/smb/client/cifsencrypt.c
index ef4c2e3c9fa6..6322f0f68a17 100644
--- a/fs/smb/client/cifsencrypt.c
+++ b/fs/smb/client/cifsencrypt.c
@@ -572,7 +572,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
 		len = cifs_strtoUTF16(user, ses->user_name, len, nls_cp);
 		UniStrupr(user);
 	} else {
-		memset(user, '\0', 2);
+		*(u16 *)user = 0;
 	}
 
 	rc = crypto_shash_update(ses->server->secmech.hmacmd5,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.6 11/21] cifs: cifs_pick_channel should try selecting active channels
       [not found] <20240202184015.540966-1-sashal@kernel.org>
  2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 10/21] smb: Work around Clang __bdos() type confusion Sasha Levin
@ 2024-02-02 18:39 ` Sasha Levin
  2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 12/21] cifs: translate network errors on send to -ECONNABORTED Sasha Levin
  2024-02-02 18:40 ` [PATCH AUTOSEL 6.6 13/21] cifs: helper function to check replayable error codes Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-02-02 18:39 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Shyam Prasad N, Steve French, Sasha Levin, sfrench, linux-cifs,
	samba-technical

From: Shyam Prasad N <sprasad@microsoft.com>

[ Upstream commit fc43a8ac396d302ced1e991e4913827cf72c8eb9 ]

cifs_pick_channel today just selects a channel based
on the policy of least loaded channel. However, it
does not take into account if the channel needs
reconnect. As a result, we can have failures in send
that can be completely avoided.

This change doesn't make a channel a candidate for
this selection if it needs reconnect.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/smb/client/transport.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index d553b7a54621..2d0a2951cbea 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -1026,6 +1026,9 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
 		if (!server)
 			continue;
 
+		if (CIFS_CHAN_NEEDS_RECONNECT(ses, i))
+			continue;
+
 		/*
 		 * strictly speaking, we should pick up req_lock to read
 		 * server->in_flight. But it shouldn't matter much here if we
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.6 12/21] cifs: translate network errors on send to -ECONNABORTED
       [not found] <20240202184015.540966-1-sashal@kernel.org>
  2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 10/21] smb: Work around Clang __bdos() type confusion Sasha Levin
  2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 11/21] cifs: cifs_pick_channel should try selecting active channels Sasha Levin
@ 2024-02-02 18:39 ` Sasha Levin
  2024-02-02 18:40 ` [PATCH AUTOSEL 6.6 13/21] cifs: helper function to check replayable error codes Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-02-02 18:39 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Shyam Prasad N, Steve French, Sasha Levin, sfrench, linux-cifs,
	samba-technical

From: Shyam Prasad N <sprasad@microsoft.com>

[ Upstream commit a68106a6928e0a6680f12bcc7338c0dddcfe4d11 ]

When the network stack returns various errors, we today bubble
up the error to the user (in case of soft mounts).

This change translates all network errors except -EINTR and
-EAGAIN to -ECONNABORTED. A similar approach is taken when
we receive network errors when reading from the socket.

The change also forces the cifsd thread to reconnect during
it's next activity.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/smb/client/transport.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 2d0a2951cbea..fee0cba264ee 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -400,10 +400,17 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
 						  server->conn_id, server->hostname);
 	}
 smbd_done:
-	if (rc < 0 && rc != -EINTR)
+	/*
+	 * there's hardly any use for the layers above to know the
+	 * actual error code here. All they should do at this point is
+	 * to retry the connection and hope it goes away.
+	 */
+	if (rc < 0 && rc != -EINTR && rc != -EAGAIN) {
 		cifs_server_dbg(VFS, "Error %d sending data on socket to server\n",
 			 rc);
-	else if (rc > 0)
+		rc = -ECONNABORTED;
+		cifs_signal_cifsd_for_reconnect(server, false);
+	} else if (rc > 0)
 		rc = 0;
 out:
 	cifs_in_send_dec(server);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.6 13/21] cifs: helper function to check replayable error codes
       [not found] <20240202184015.540966-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 12/21] cifs: translate network errors on send to -ECONNABORTED Sasha Levin
@ 2024-02-02 18:40 ` Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2024-02-02 18:40 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Shyam Prasad N, Steve French, Sasha Levin, sfrench, linux-cifs,
	samba-technical

From: Shyam Prasad N <sprasad@microsoft.com>

[ Upstream commit 64cc377b7628b81ffdbdb1c6bacfba895dcac3f8 ]

The code to check for replay is not just -EAGAIN. In some
cases, the send request or receive response may result in
network errors, which we're now mapping to -ECONNABORTED.

This change introduces a helper function which checks
if the error returned in one of the above two errors.
And all checks for replays will now use this helper.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/smb/client/cached_dir.c | 1 +
 fs/smb/client/cifsglob.h   | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index d64a306a414b..ef96d5f7809e 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -367,6 +367,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 		atomic_inc(&tcon->num_remote_opens);
 	}
 	kfree(utf16_path);
+
 	return rc;
 }
 
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index ec1e5e20a36b..000ed4a01624 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1804,6 +1804,13 @@ static inline bool is_retryable_error(int error)
 	return false;
 }
 
+static inline bool is_replayable_error(int error)
+{
+	if (error == -EAGAIN || error == -ECONNABORTED)
+		return true;
+	return false;
+}
+
 
 /* cifs_get_writable_file() flags */
 #define FIND_WR_ANY         0
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-02 18:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20240202184015.540966-1-sashal@kernel.org>
2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 10/21] smb: Work around Clang __bdos() type confusion Sasha Levin
2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 11/21] cifs: cifs_pick_channel should try selecting active channels Sasha Levin
2024-02-02 18:39 ` [PATCH AUTOSEL 6.6 12/21] cifs: translate network errors on send to -ECONNABORTED Sasha Levin
2024-02-02 18:40 ` [PATCH AUTOSEL 6.6 13/21] cifs: helper function to check replayable error codes Sasha Levin

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).