diff mbox series

[SMB3] workaround negprot bug in some Samba servers by changing order of negcontexts sent by Linux kernel client

Message ID CAH2r5mtuN-yswT5VTbNPzj02fwiHYOCe2eR8mcgRgRE8Qpkjgw@mail.gmail.com
State New
Headers show
Series [SMB3] workaround negprot bug in some Samba servers by changing order of negcontexts sent by Linux kernel client | expand

Commit Message

Steve French July 12, 2022, 5:34 a.m. UTC 
Starting with 5.18.8 (and 5.19-rc4) mount can now fail to older Samba
servers due to a server bug handling padding at the end of the last
negotiate context (negotiate contexts typically round up to 8 byte
lengths by adding padding if needed). This server bug can be avoided
by switching the order of negotiate contexts, placing a negotiate
context at the end that does not require padding (prior to the recent
netname context fix this was the case on the client).

Fixes: 73130a7b1ac9 ("smb3: fix empty netname context on secondary channels")

See attached fix to cifs.ko

Comments

Shyam Prasad N July 12, 2022, 7:04 a.m. UTC | #1 
On Tue, Jul 12, 2022 at 11:04 AM Steve French <smfrench@gmail.com> wrote:
>
> Starting with 5.18.8 (and 5.19-rc4) mount can now fail to older Samba
> servers due to a server bug handling padding at the end of the last
> negotiate context (negotiate contexts typically round up to 8 byte
> lengths by adding padding if needed). This server bug can be avoided
> by switching the order of negotiate contexts, placing a negotiate
> context at the end that does not require padding (prior to the recent
> netname context fix this was the case on the client).
>
> Fixes: 73130a7b1ac9 ("smb3: fix empty netname context on secondary channels")
>
> See attached fix to cifs.ko
> --
> Thanks,
>
> Steve

Looks good to me.
Julian Sikorski July 13, 2022, 2:57 p.m. UTC | #2 
Am 12.07.22 um 07:34 schrieb Steve French:
> Starting with 5.18.8 (and 5.19-rc4) mount can now fail to older Samba
> servers due to a server bug handling padding at the end of the last
> negotiate context (negotiate contexts typically round up to 8 byte
> lengths by adding padding if needed). This server bug can be avoided
> by switching the order of negotiate contexts, placing a negotiate
> context at the end that does not require padding (prior to the recent
> netname context fix this was the case on the client).
> 
> Fixes: 73130a7b1ac9 ("smb3: fix empty netname context on secondary channels")
> 
> See attached fix to cifs.ko

This patch fixes the issue for me when applied on top of 5.18.11. Thanks!

Best regards,
Julian

Tested-by: Julian Sikorski <belegdol+github@gmail.com>
diff mbox series

Patch

From a8d8532e4c335f0a31dd213abe4e31682f34647c Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Tue, 12 Jul 2022 00:11:42 -0500
Subject: [PATCH] smb3: workaround negprot bug in some Samba servers

Mount can now fail to older Samba servers due to a server
bug handling padding at the end of the last negotiate
contexts (negotiate contexts typically round up to 8 byte
lengths by adding padding if needed). This server bug can
be avoided by switching the order of negotiate contexts,
placing a negotiate context at the end that does not
require padding (prior to the recent netname context fix
this was the case on the client).

Fixes: 73130a7b1ac9 ("smb3: fix empty netname context on secondary channels")
Reported-by: Julian Sikorski <belegdol@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2pdu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 12b4dddaedb0..c705de32e225 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -571,10 +571,6 @@  assemble_neg_contexts(struct smb2_negotiate_req *req,
 	*total_len += ctxt_len;
 	pneg_ctxt += ctxt_len;
 
-	build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
-	*total_len += sizeof(struct smb2_posix_neg_context);
-	pneg_ctxt += sizeof(struct smb2_posix_neg_context);
-
 	/*
 	 * secondary channels don't have the hostname field populated
 	 * use the hostname field in the primary channel instead
@@ -586,9 +582,14 @@  assemble_neg_contexts(struct smb2_negotiate_req *req,
 					      hostname);
 		*total_len += ctxt_len;
 		pneg_ctxt += ctxt_len;
-		neg_context_count = 4;
-	} else /* second channels do not have a hostname */
 		neg_context_count = 3;
+	} else
+		neg_context_count = 2;
+
+	build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
+	*total_len += sizeof(struct smb2_posix_neg_context);
+	pneg_ctxt += sizeof(struct smb2_posix_neg_context);
+	neg_context_count++;
 
 	if (server->compress_algorithm) {
 		build_compression_ctxt((struct smb2_compression_capabilities_context *)
-- 
2.34.1