Re: [Linux-stable-mirror] [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
4.13-stable review patch. If anyone has any objections, please let me know.
From: Steve French smfrench@gmail.com
commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
According to MS-SMB2 3.2.55 validate_negotiate request must always be signed. Some Windows can fail the request if you send it unsigned
See kernel bugzilla bug 197311
Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> Signed-off-by: Steve French smfrench@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/cifs/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+)
--- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc } else iov[0].iov_len = get_rfc1002_length(req) + 4;
- /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
- if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;This one needs to be backported to all stable kernels as the commit that introduced the regression: ' 0603c96f3af50e2f9299fa410c224ab1d465e0f9 SMB: Validate negotiate (to protect against downgrade) even if signing off
is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
Oh wait, it breaks the builds on older kernels, that's why I didn't apply it :)
Can you provide me with a working backport?
Hi Steve,
Is there a version of this fix available for stable kernels?
I tried applying this patch to 4.4.109 (and a similar one for 4.9.74), but it didn't fix the problem. Instead, I actually got a NULL pointer dereference when I tried to mount an SMB3 share.
Here is the patch I tried on 4.4.109:
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index f2ff60e..3963bd2 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1559,6 +1559,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, } else iov[0].iov_len = get_rfc1002_length(req) + 4;
+ /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) + req->hdr.Flags |= SMB2_FLAGS_SIGNED;
rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0); rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
This results in the following NULL pointer dereference when I try mounting:
# mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir
[ 53.073057] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050 [ 53.073511] IP: [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0 [ 53.073973] PGD 0 [ 53.074427] Oops: 0000 [#1] SMP [ 53.074946] Modules linked in: arc4(E) ecb(E) md4(E) cifs(E) dns_resolver(E) vmw_vsock_vmci_transport(E) vsock(E) hid_generic(E) usbhid(E) hid(E) xt_conntrack(E) mousedev(E) iptable_nat(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) nf_nat_ipv4(E) nf_nat(E) iptable_filter(E) ip_tables(E) crc32c_intel(E) xt_LOG(E) nf_conntrack(E) jitterentropy_rng(E) hmac(E) sha256_ssse3(E) sha256_generic(E) drbg(E) vmw_balloon(E) ansi_cprng(E) aesni_intel(E) aes_x86_64(E) glue_helper(E) lrw(E) gf128mul(E) ablk_helper(E) cryptd(E) psmouse(E) evdev(E) uhci_hcd(E) ehci_pci(E) ehci_hcd(E) usbcore(E) intel_agp(E) usb_common(E) vmw_vmci(E) i2c_piix4(E) intel_gtt(E) nfit(E) battery(E) tpm_tis(E) tpm(E) ac(E) button(E) sch_fq_codel(E) autofs4(E) [ 53.079435] CPU: 3 PID: 829 Comm: mount.cifs Tainted: G E 4.4.109-possible-fix1+ #21 [ 53.079983] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016 [ 53.081086] task: ffff8800b4f41940 ti: ffff8800b92ac000 task.ti: ffff8800b92ac000 [ 53.081667] RIP: 0010:[<ffffffff8138ee9a>] [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0 [ 53.082247] RSP: 0018:ffff8800b92af9a8 EFLAGS: 00010282 [ 53.082604] systemd-journald[284]: Compressed data object 721 -> 468 using XZ [ 53.083419] RAX: ffff8800af5943c0 RBX: ffff8800b484a800 RCX: 00000000ffff0ec7 [ 53.084001] RDX: 0000000000000010 RSI: ffff8800b900af18 RDI: 0000000000000000 [ 53.084602] RBP: ffff8800b92af9e0 R08: ffff8800b92afb64 R09: 0000000000000000 [ 53.085184] R10: 3031322e3030312e R11: 00000000000007f5 R12: 0000000000000002 [ 53.085755] R13: 0000000000000000 R14: ffff8800b900af18 R15: 0000000000000010 [ 53.086333] FS: 00007fb659b45740(0000) GS:ffff88013fcc0000(0000) knlGS:0000000000000000 [ 53.086907] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 53.087480] CR2: 0000000000000050 CR3: 00000000b7970000 CR4: 00000000001606e0 [ 53.088107] Stack: [ 53.088681] ffff8800bba5d8c0 ffff8800b92afa08 ffff8800b484a800 0000000000000002 [ 53.089281] ffff8800b92afac8 000008012400007d ffff8800b484a800 ffff8800b92afa50 [ 53.089871] ffffffffa02194a6 ffff8800b92afb70 ffff8800af5943c0 ffff8800b7a2f800 [ 53.090457] Call Trace: [ 53.091054] [<ffffffffa02194a6>] smb3_calc_signature+0xb6/0x290 [cifs] [ 53.091650] [<ffffffffa0218b5b>] smb2_sign_rqst+0x2b/0x40 [cifs] [ 53.092244] [<ffffffffa0219981>] smb2_setup_request+0xd1/0x170 [cifs] [ 53.092838] [<ffffffffa02082a7>] SendReceive2+0xc7/0x450 [cifs] [ 53.093435] [<ffffffffa02057d5>] ? cifs_small_buf_get+0x15/0x30 [cifs] [ 53.094030] [<ffffffffa021b83f>] ? small_smb2_init+0xdf/0x200 [cifs] [ 53.094616] [<ffffffffa021d6d7>] SMB2_ioctl+0x147/0x310 [cifs] [ 53.095203] [<ffffffffa021d99e>] smb3_validate_negotiate+0xfe/0x2d0 [cifs] [ 53.095792] [<ffffffffa021b196>] SMB2_tcon+0x296/0x500 [cifs] [ 53.096362] [<ffffffff817d7b49>] ? _raw_spin_unlock_irqrestore+0x9/0x10 [ 53.096930] [<ffffffffa01efe0b>] cifs_get_tcon+0x1bb/0x560 [cifs] [ 53.097486] [<ffffffffa01f2b10>] cifs_mount+0x690/0xde0 [cifs] [ 53.098032] [<ffffffff817d7b49>] ? _raw_spin_unlock_irqrestore+0x9/0x10 [ 53.098570] [<ffffffffa01de6eb>] cifs_do_mount+0xcb/0x5a0 [cifs] [ 53.099089] [<ffffffff81193ef7>] ? alloc_pages_current+0x87/0x110 [ 53.099598] [<ffffffff811b7b03>] mount_fs+0x33/0x160 [ 53.100091] [<ffffffff811d1b62>] vfs_kern_mount+0x62/0x100 [ 53.100574] [<ffffffff811d3f1b>] do_mount+0x21b/0xd30 [ 53.101050] [<ffffffff81193ef7>] ? alloc_pages_current+0x87/0x110 [ 53.101511] [<ffffffff811d4d47>] SyS_mount+0x87/0xd0 [ 53.101959] [<ffffffff817d806e>] entry_SYSCALL_64_fastpath+0x12/0x71 [ 53.102400] Code: 89 e5 8b 12 e8 78 d2 04 00 31 c0 5d c3 0f 1f 40 00 55 48 89 e5 41 57 41 56 41 55 41 54 49 89 fd 53 49 89 f6 41 89 d7 48 83 ec 10 <4c> 8b 67 50 41 8b 5c 24 2c 48 85 de 75 14 41 ff 54 24 e8 48 83 [ 53.103820] RIP [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0 [ 53.104288] RSP <ffff8800b92af9a8> [ 53.104745] CR2: 0000000000000050 [ 53.105225] ---[ end trace fc2de0ad7f229314 ]---
The CIFS config options enabled are:
CONFIG_CIFS=m CONFIG_CIFS_STATS=y CONFIG_CIFS_STATS2=y CONFIG_CIFS_WEAK_PW_HASH=y CONFIG_CIFS_UPCALL=y CONFIG_CIFS_XATTR=y CONFIG_CIFS_POSIX=y CONFIG_CIFS_ACL=y CONFIG_CIFS_DEBUG=y CONFIG_CIFS_DEBUG2=y CONFIG_CIFS_DFS_UPCALL=y CONFIG_CIFS_SMB2=y # CONFIG_CIFS_SMB311 is not set # CONFIG_CIFS_FSCACHE is not set
The problem seems to be that crypto_shash_setkey() is called without calling smb3_crypto_shash_allocate() first. So I looked up how mainline avoids this issue, and it looks like the following commit makes a call to generate_signingkey() even when server->sign == false, and this path eventually calls smb3_crytpto_shash_allocate()), thus avoiding the NULL pointer dereference.
cabfb3680f78 (CIFS: Enable encryption during session setup phase)
So, I adopted this change, and now my resulting patch looks like this:
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index f2ff60e..19cc92c 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -833,7 +833,7 @@ ssetup_exit:
if (!rc) { mutex_lock(&server->srv_mutex); - if (server->sign && server->ops->generate_signingkey) { + if (server->ops->generate_signingkey) { rc = server->ops->generate_signingkey(ses); kfree(ses->auth_key.response); ses->auth_key.response = NULL; @@ -1559,6 +1559,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, } else iov[0].iov_len = get_rfc1002_length(req) + 4;
+ /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) + req->hdr.Flags |= SMB2_FLAGS_SIGNED;
rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0); rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
This fixes the NULL pointer dereference, but the mount still fails, but this time for a different reason -- due to STATUS_ACCESS_DENIED:
# mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir
mount.cifs kernel mount options: ip=<ip_addr>,unc=\<ip_addr>\TestSMB,vers=3.0,user=srivatsa,pass=******** mount error(5): Input/output error Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
Here is the dmesg output:
[ 48.192141] fs/cifs/cifsfs.c: Devname: //<ip_addr>/TestSMB/ flags: 0 [ 48.192178] address conversion returned 1 for <ip_addr> [ 48.192205] fs/cifs/connect.c: Username: srivatsa [ 48.192222] fs/cifs/connect.c: file mode: 0x1ed dir mode: 0x1ed [ 48.192280] fs/cifs/connect.c: CIFS VFS: in cifs_mount as Xid: 0 with uid: 0 [ 48.192302] fs/cifs/connect.c: UNC: \<ip_addr>\TestSMB [ 48.192335] fs/cifs/connect.c: Socket created [ 48.192349] fs/cifs/connect.c: sndbuf 16384 rcvbuf 87380 rcvtimeo 0x6d6 [ 48.193453] fs/cifs/connect.c: CIFS VFS: in cifs_get_smb_ses as Xid: 1 with uid: 0 [ 48.193462] fs/cifs/connect.c: Demultiplex PID: 829 [ 48.193492] fs/cifs/connect.c: Existing smb sess not found [ 48.193510] fs/cifs/smb2pdu.c: Negotiate protocol [ 48.193531] fs/cifs/transport.c: Sending smb: smb_len=102 [ 48.194301] fs/cifs/connect.c: RFC1002 header 0xaa [ 48.194321] fs/cifs/smb2misc.c: smb2_check_message length: 0xae, smb_buf_length: 0xaa [ 48.194349] fs/cifs/smb2misc.c: SMB2 data length 42 offset 128 [ 48.194367] fs/cifs/smb2misc.c: SMB2 len 174 [ 48.194393] fs/cifs/transport.c: cifs_sync_mid_result: cmd=0 mid=0 state=4 [ 48.194415] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.194436] fs/cifs/smb2pdu.c: mode 0x1 [ 48.194448] fs/cifs/smb2pdu.c: negotiated smb3.0 dialect [ 48.194466] fs/cifs/asn1.c: OID len = 10 oid = 0x1 0x3 0x6 0x1 [ 48.194484] fs/cifs/asn1.c: OID len = 10 oid = 0x1 0x3 0x6 0x1 [ 48.194502] fs/cifs/connect.c: Security Mode: 0x1 Capabilities: 0x300007 TimeAdjust: 0 [ 48.194525] fs/cifs/smb2pdu.c: Session Setup [ 48.194539] fs/cifs/transport.c: Sending smb: smb_len=120 [ 48.194817] fs/cifs/connect.c: RFC1002 header 0x136 [ 48.194836] fs/cifs/smb2misc.c: smb2_check_message length: 0x13a, smb_buf_length: 0x136 [ 48.194859] fs/cifs/smb2misc.c: SMB2 data length 238 offset 72 [ 48.195306] fs/cifs/smb2misc.c: SMB2 len 314 [ 48.195740] fs/cifs/transport.c: cifs_sync_mid_result: cmd=1 mid=1 state=4 [ 48.196174] Status code returned 0xc0000016 STATUS_MORE_PROCESSING_REQUIRED [ 48.196605] fs/cifs/smb2maperror.c: Mapping SMB2 status code -1073741802 to POSIX err -5 [ 48.197043] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.210008] fs/cifs/transport.c: Sending smb: smb_len=412 [ 48.211625] fs/cifs/connect.c: RFC1002 header 0x48 [ 48.212060] fs/cifs/smb2misc.c: smb2_check_message length: 0x4c, smb_buf_length: 0x48 [ 48.212494] fs/cifs/smb2misc.c: SMB2 data length 0 offset 72 [ 48.212919] fs/cifs/smb2misc.c: SMB2 len 77 [ 48.213364] fs/cifs/smb2misc.c: Calculated size 77 length 76 mismatch mid 2 [ 48.213807] fs/cifs/transport.c: cifs_sync_mid_result: cmd=1 mid=2 state=4 [ 48.214242] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.219385] fs/cifs/smb2pdu.c: SMB2/3 session established successfully [ 48.219831] fs/cifs/connect.c: CIFS VFS: leaving cifs_get_smb_ses (xid = 1) rc = 0 [ 48.220276] fs/cifs/connect.c: CIFS VFS: in cifs_get_tcon as Xid: 2 with uid: 0 [ 48.220724] fs/cifs/smb2pdu.c: TCON [ 48.221182] fs/cifs/transport.c: Sending smb: smb_len=122 [ 48.221830] fs/cifs/connect.c: RFC1002 header 0x50 [ 48.222280] fs/cifs/smb2misc.c: smb2_check_message length: 0x54, smb_buf_length: 0x50 [ 48.222734] fs/cifs/smb2misc.c: SMB2 len 84 [ 48.223199] fs/cifs/transport.c: cifs_sync_mid_result: cmd=3 mid=3 state=4 [ 48.223656] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.224107] fs/cifs/smb2pdu.c: connection to disk share [ 48.224560] fs/cifs/smb2pdu.c: validate negotiate [ 48.225015] fs/cifs/smb2pdu.c: SMB2 IOCTL [ 48.225456] fs/cifs/transport.c: Sending smb: smb_len=146 [ 48.226049] fs/cifs/connect.c: RFC1002 header 0x49 [ 48.226498] fs/cifs/smb2misc.c: smb2_check_message length: 0x4d, smb_buf_length: 0x49 [ 48.226951] fs/cifs/smb2misc.c: SMB2 data length 0 offset 0 [ 48.227408] fs/cifs/smb2misc.c: SMB2 len 77 [ 48.227863] fs/cifs/transport.c: cifs_sync_mid_result: cmd=11 mid=4 state=4 [ 48.228318] Status code returned 0xc0000022 STATUS_ACCESS_DENIED [ 48.228780] fs/cifs/smb2maperror.c: Mapping SMB2 status code -1073741790 to POSIX err -13 [ 48.229265] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.229732] CIFS VFS: validate protocol negotiate failed: -13 [ 48.230197] fs/cifs/connect.c: CIFS VFS: leaving cifs_get_tcon (xid = 2) rc = -5 [ 48.230681] fs/cifs/connect.c: Tcon rc = -5 [ 48.231150] fs/cifs/connect.c: build_unc_path_to_root: full_path=\<ip_addr>\TestSMB [ 48.231634] fs/cifs/connect.c: cifs_put_smb_ses: ses_count=1 [ 48.232101] fs/cifs/connect.c: CIFS VFS: in cifs_put_smb_ses as Xid: 3 with uid: 0 [ 48.232569] fs/cifs/smb2pdu.c: disconnect session ffff8800b9189e00 [ 48.233053] fs/cifs/transport.c: Sending smb: smb_len=68 [ 48.233651] fs/cifs/connect.c: RFC1002 header 0x44 [ 48.234116] fs/cifs/smb2misc.c: smb2_check_message length: 0x48, smb_buf_length: 0x44 [ 48.234585] fs/cifs/smb2misc.c: SMB2 len 72 [ 48.235063] fs/cifs/transport.c: cifs_sync_mid_result: cmd=2 mid=5 state=4 [ 48.235541] SendRcvNoRsp flags 64 rc 0 [ 48.236075] fs/cifs/connect.c: CIFS VFS: leaving cifs_mount (xid = 0) rc = -5 [ 48.236556] CIFS VFS: cifs_mount failed w/return code = -5
Any thoughts on what is the right fix for stable kernels? Mounting SMB3 shares works great on mainline (v4.15-rc5). It also works on 4.4.109 if I pass the sec=ntlmsspi option to the mount command (as opposed to the default: sec=ntlmssp). Please let me know if you need any other info.
Thank you!
Regards, Srivatsa
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
4.13-stable review patch. If anyone has any objections, please let me know.
From: Steve French smfrench@gmail.com
commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
According to MS-SMB2 3.2.55 validate_negotiate request must always be signed. Some Windows can fail the request if you send it unsigned
See kernel bugzilla bug 197311
Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> Signed-off-by: Steve French smfrench@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/cifs/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+)
--- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc } else iov[0].iov_len = get_rfc1002_length(req) + 4;
- /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
- if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;This one needs to be backported to all stable kernels as the commit that introduced the regression: ' 0603c96f3af50e2f9299fa410c224ab1d465e0f9 SMB: Validate negotiate (to protect against downgrade) even if signing off
is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
Oh wait, it breaks the builds on older kernels, that's why I didn't apply it :)
Can you provide me with a working backport?
Hi Steve,
Is there a version of this fix available for stable kernels?
Any thoughts on this?
Regards, Srivatsa
I tried applying this patch to 4.4.109 (and a similar one for 4.9.74), but it didn't fix the problem. Instead, I actually got a NULL pointer dereference when I tried to mount an SMB3 share.
Here is the patch I tried on 4.4.109:
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index f2ff60e..3963bd2 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1559,6 +1559,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, } else iov[0].iov_len = get_rfc1002_length(req) + 4;
/* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)req->hdr.Flags |= SMB2_FLAGS_SIGNED;rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0); rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
This results in the following NULL pointer dereference when I try mounting:
# mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir
[ 53.073057] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050 [ 53.073511] IP: [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0 [ 53.073973] PGD 0 [ 53.074427] Oops: 0000 [#1] SMP [ 53.074946] Modules linked in: arc4(E) ecb(E) md4(E) cifs(E) dns_resolver(E) vmw_vsock_vmci_transport(E) vsock(E) hid_generic(E) usbhid(E) hid(E) xt_conntrack(E) mousedev(E) iptable_nat(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) nf_nat_ipv4(E) nf_nat(E) iptable_filter(E) ip_tables(E) crc32c_intel(E) xt_LOG(E) nf_conntrack(E) jitterentropy_rng(E) hmac(E) sha256_ssse3(E) sha256_generic(E) drbg(E) vmw_balloon(E) ansi_cprng(E) aesni_intel(E) aes_x86_64(E) glue_helper(E) lrw(E) gf128mul(E) ablk_helper(E) cryptd(E) psmouse(E) evdev(E) uhci_hcd(E) ehci_pci(E) ehci_hcd(E) usbcore(E) intel_agp(E) usb_common(E) vmw_vmci(E) i2c_piix4(E) intel_gtt(E) nfit(E) battery(E) tpm_tis(E) tpm(E) ac(E) button(E) sch_fq_codel(E) autofs4(E) [ 53.079435] CPU: 3 PID: 829 Comm: mount.cifs Tainted: G E 4.4.109-possible-fix1+ #21 [ 53.079983] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016 [ 53.081086] task: ffff8800b4f41940 ti: ffff8800b92ac000 task.ti: ffff8800b92ac000 [ 53.081667] RIP: 0010:[<ffffffff8138ee9a>] [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0 [ 53.082247] RSP: 0018:ffff8800b92af9a8 EFLAGS: 00010282 [ 53.082604] systemd-journald[284]: Compressed data object 721 -> 468 using XZ [ 53.083419] RAX: ffff8800af5943c0 RBX: ffff8800b484a800 RCX: 00000000ffff0ec7 [ 53.084001] RDX: 0000000000000010 RSI: ffff8800b900af18 RDI: 0000000000000000 [ 53.084602] RBP: ffff8800b92af9e0 R08: ffff8800b92afb64 R09: 0000000000000000 [ 53.085184] R10: 3031322e3030312e R11: 00000000000007f5 R12: 0000000000000002 [ 53.085755] R13: 0000000000000000 R14: ffff8800b900af18 R15: 0000000000000010 [ 53.086333] FS: 00007fb659b45740(0000) GS:ffff88013fcc0000(0000) knlGS:0000000000000000 [ 53.086907] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 53.087480] CR2: 0000000000000050 CR3: 00000000b7970000 CR4: 00000000001606e0 [ 53.088107] Stack: [ 53.088681] ffff8800bba5d8c0 ffff8800b92afa08 ffff8800b484a800 0000000000000002 [ 53.089281] ffff8800b92afac8 000008012400007d ffff8800b484a800 ffff8800b92afa50 [ 53.089871] ffffffffa02194a6 ffff8800b92afb70 ffff8800af5943c0 ffff8800b7a2f800 [ 53.090457] Call Trace: [ 53.091054] [<ffffffffa02194a6>] smb3_calc_signature+0xb6/0x290 [cifs] [ 53.091650] [<ffffffffa0218b5b>] smb2_sign_rqst+0x2b/0x40 [cifs] [ 53.092244] [<ffffffffa0219981>] smb2_setup_request+0xd1/0x170 [cifs] [ 53.092838] [<ffffffffa02082a7>] SendReceive2+0xc7/0x450 [cifs] [ 53.093435] [<ffffffffa02057d5>] ? cifs_small_buf_get+0x15/0x30 [cifs] [ 53.094030] [<ffffffffa021b83f>] ? small_smb2_init+0xdf/0x200 [cifs] [ 53.094616] [<ffffffffa021d6d7>] SMB2_ioctl+0x147/0x310 [cifs] [ 53.095203] [<ffffffffa021d99e>] smb3_validate_negotiate+0xfe/0x2d0 [cifs] [ 53.095792] [<ffffffffa021b196>] SMB2_tcon+0x296/0x500 [cifs] [ 53.096362] [<ffffffff817d7b49>] ? _raw_spin_unlock_irqrestore+0x9/0x10 [ 53.096930] [<ffffffffa01efe0b>] cifs_get_tcon+0x1bb/0x560 [cifs] [ 53.097486] [<ffffffffa01f2b10>] cifs_mount+0x690/0xde0 [cifs] [ 53.098032] [<ffffffff817d7b49>] ? _raw_spin_unlock_irqrestore+0x9/0x10 [ 53.098570] [<ffffffffa01de6eb>] cifs_do_mount+0xcb/0x5a0 [cifs] [ 53.099089] [<ffffffff81193ef7>] ? alloc_pages_current+0x87/0x110 [ 53.099598] [<ffffffff811b7b03>] mount_fs+0x33/0x160 [ 53.100091] [<ffffffff811d1b62>] vfs_kern_mount+0x62/0x100 [ 53.100574] [<ffffffff811d3f1b>] do_mount+0x21b/0xd30 [ 53.101050] [<ffffffff81193ef7>] ? alloc_pages_current+0x87/0x110 [ 53.101511] [<ffffffff811d4d47>] SyS_mount+0x87/0xd0 [ 53.101959] [<ffffffff817d806e>] entry_SYSCALL_64_fastpath+0x12/0x71 [ 53.102400] Code: 89 e5 8b 12 e8 78 d2 04 00 31 c0 5d c3 0f 1f 40 00 55 48 89 e5 41 57 41 56 41 55 41 54 49 89 fd 53 49 89 f6 41 89 d7 48 83 ec 10 <4c> 8b 67 50 41 8b 5c 24 2c 48 85 de 75 14 41 ff 54 24 e8 48 83 [ 53.103820] RIP [<ffffffff8138ee9a>] crypto_shash_setkey+0x1a/0xc0 [ 53.104288] RSP <ffff8800b92af9a8> [ 53.104745] CR2: 0000000000000050 [ 53.105225] ---[ end trace fc2de0ad7f229314 ]---
The CIFS config options enabled are:
CONFIG_CIFS=m CONFIG_CIFS_STATS=y CONFIG_CIFS_STATS2=y CONFIG_CIFS_WEAK_PW_HASH=y CONFIG_CIFS_UPCALL=y CONFIG_CIFS_XATTR=y CONFIG_CIFS_POSIX=y CONFIG_CIFS_ACL=y CONFIG_CIFS_DEBUG=y CONFIG_CIFS_DEBUG2=y CONFIG_CIFS_DFS_UPCALL=y CONFIG_CIFS_SMB2=y # CONFIG_CIFS_SMB311 is not set # CONFIG_CIFS_FSCACHE is not set
The problem seems to be that crypto_shash_setkey() is called without calling smb3_crypto_shash_allocate() first. So I looked up how mainline avoids this issue, and it looks like the following commit makes a call to generate_signingkey() even when server->sign == false, and this path eventually calls smb3_crytpto_shash_allocate()), thus avoiding the NULL pointer dereference.
cabfb3680f78 (CIFS: Enable encryption during session setup phase)
So, I adopted this change, and now my resulting patch looks like this:
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index f2ff60e..19cc92c 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -833,7 +833,7 @@ ssetup_exit: if (!rc) { mutex_lock(&server->srv_mutex);
if (server->sign && server->ops->generate_signingkey) {
if (server->ops->generate_signingkey) { rc = server->ops->generate_signingkey(ses); kfree(ses->auth_key.response); ses->auth_key.response = NULL;@@ -1559,6 +1559,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, } else iov[0].iov_len = get_rfc1002_length(req) + 4;
/* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)req->hdr.Flags |= SMB2_FLAGS_SIGNED;rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0); rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
This fixes the NULL pointer dereference, but the mount still fails, but this time for a different reason -- due to STATUS_ACCESS_DENIED:
# mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir
mount.cifs kernel mount options: ip=<ip_addr>,unc=\<ip_addr>\TestSMB,vers=3.0,user=srivatsa,pass=******** mount error(5): Input/output error Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
Here is the dmesg output:
[ 48.192141] fs/cifs/cifsfs.c: Devname: //<ip_addr>/TestSMB/ flags: 0 [ 48.192178] address conversion returned 1 for <ip_addr> [ 48.192205] fs/cifs/connect.c: Username: srivatsa [ 48.192222] fs/cifs/connect.c: file mode: 0x1ed dir mode: 0x1ed [ 48.192280] fs/cifs/connect.c: CIFS VFS: in cifs_mount as Xid: 0 with uid: 0 [ 48.192302] fs/cifs/connect.c: UNC: \<ip_addr>\TestSMB [ 48.192335] fs/cifs/connect.c: Socket created [ 48.192349] fs/cifs/connect.c: sndbuf 16384 rcvbuf 87380 rcvtimeo 0x6d6 [ 48.193453] fs/cifs/connect.c: CIFS VFS: in cifs_get_smb_ses as Xid: 1 with uid: 0 [ 48.193462] fs/cifs/connect.c: Demultiplex PID: 829 [ 48.193492] fs/cifs/connect.c: Existing smb sess not found [ 48.193510] fs/cifs/smb2pdu.c: Negotiate protocol [ 48.193531] fs/cifs/transport.c: Sending smb: smb_len=102 [ 48.194301] fs/cifs/connect.c: RFC1002 header 0xaa [ 48.194321] fs/cifs/smb2misc.c: smb2_check_message length: 0xae, smb_buf_length: 0xaa [ 48.194349] fs/cifs/smb2misc.c: SMB2 data length 42 offset 128 [ 48.194367] fs/cifs/smb2misc.c: SMB2 len 174 [ 48.194393] fs/cifs/transport.c: cifs_sync_mid_result: cmd=0 mid=0 state=4 [ 48.194415] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.194436] fs/cifs/smb2pdu.c: mode 0x1 [ 48.194448] fs/cifs/smb2pdu.c: negotiated smb3.0 dialect [ 48.194466] fs/cifs/asn1.c: OID len = 10 oid = 0x1 0x3 0x6 0x1 [ 48.194484] fs/cifs/asn1.c: OID len = 10 oid = 0x1 0x3 0x6 0x1 [ 48.194502] fs/cifs/connect.c: Security Mode: 0x1 Capabilities: 0x300007 TimeAdjust: 0 [ 48.194525] fs/cifs/smb2pdu.c: Session Setup [ 48.194539] fs/cifs/transport.c: Sending smb: smb_len=120 [ 48.194817] fs/cifs/connect.c: RFC1002 header 0x136 [ 48.194836] fs/cifs/smb2misc.c: smb2_check_message length: 0x13a, smb_buf_length: 0x136 [ 48.194859] fs/cifs/smb2misc.c: SMB2 data length 238 offset 72 [ 48.195306] fs/cifs/smb2misc.c: SMB2 len 314 [ 48.195740] fs/cifs/transport.c: cifs_sync_mid_result: cmd=1 mid=1 state=4 [ 48.196174] Status code returned 0xc0000016 STATUS_MORE_PROCESSING_REQUIRED [ 48.196605] fs/cifs/smb2maperror.c: Mapping SMB2 status code -1073741802 to POSIX err -5 [ 48.197043] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.210008] fs/cifs/transport.c: Sending smb: smb_len=412 [ 48.211625] fs/cifs/connect.c: RFC1002 header 0x48 [ 48.212060] fs/cifs/smb2misc.c: smb2_check_message length: 0x4c, smb_buf_length: 0x48 [ 48.212494] fs/cifs/smb2misc.c: SMB2 data length 0 offset 72 [ 48.212919] fs/cifs/smb2misc.c: SMB2 len 77 [ 48.213364] fs/cifs/smb2misc.c: Calculated size 77 length 76 mismatch mid 2 [ 48.213807] fs/cifs/transport.c: cifs_sync_mid_result: cmd=1 mid=2 state=4 [ 48.214242] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.219385] fs/cifs/smb2pdu.c: SMB2/3 session established successfully [ 48.219831] fs/cifs/connect.c: CIFS VFS: leaving cifs_get_smb_ses (xid = 1) rc = 0 [ 48.220276] fs/cifs/connect.c: CIFS VFS: in cifs_get_tcon as Xid: 2 with uid: 0 [ 48.220724] fs/cifs/smb2pdu.c: TCON [ 48.221182] fs/cifs/transport.c: Sending smb: smb_len=122 [ 48.221830] fs/cifs/connect.c: RFC1002 header 0x50 [ 48.222280] fs/cifs/smb2misc.c: smb2_check_message length: 0x54, smb_buf_length: 0x50 [ 48.222734] fs/cifs/smb2misc.c: SMB2 len 84 [ 48.223199] fs/cifs/transport.c: cifs_sync_mid_result: cmd=3 mid=3 state=4 [ 48.223656] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.224107] fs/cifs/smb2pdu.c: connection to disk share [ 48.224560] fs/cifs/smb2pdu.c: validate negotiate [ 48.225015] fs/cifs/smb2pdu.c: SMB2 IOCTL [ 48.225456] fs/cifs/transport.c: Sending smb: smb_len=146 [ 48.226049] fs/cifs/connect.c: RFC1002 header 0x49 [ 48.226498] fs/cifs/smb2misc.c: smb2_check_message length: 0x4d, smb_buf_length: 0x49 [ 48.226951] fs/cifs/smb2misc.c: SMB2 data length 0 offset 0 [ 48.227408] fs/cifs/smb2misc.c: SMB2 len 77 [ 48.227863] fs/cifs/transport.c: cifs_sync_mid_result: cmd=11 mid=4 state=4 [ 48.228318] Status code returned 0xc0000022 STATUS_ACCESS_DENIED [ 48.228780] fs/cifs/smb2maperror.c: Mapping SMB2 status code -1073741790 to POSIX err -13 [ 48.229265] fs/cifs/misc.c: Null buffer passed to cifs_small_buf_release [ 48.229732] CIFS VFS: validate protocol negotiate failed: -13 [ 48.230197] fs/cifs/connect.c: CIFS VFS: leaving cifs_get_tcon (xid = 2) rc = -5 [ 48.230681] fs/cifs/connect.c: Tcon rc = -5 [ 48.231150] fs/cifs/connect.c: build_unc_path_to_root: full_path=\<ip_addr>\TestSMB [ 48.231634] fs/cifs/connect.c: cifs_put_smb_ses: ses_count=1 [ 48.232101] fs/cifs/connect.c: CIFS VFS: in cifs_put_smb_ses as Xid: 3 with uid: 0 [ 48.232569] fs/cifs/smb2pdu.c: disconnect session ffff8800b9189e00 [ 48.233053] fs/cifs/transport.c: Sending smb: smb_len=68 [ 48.233651] fs/cifs/connect.c: RFC1002 header 0x44 [ 48.234116] fs/cifs/smb2misc.c: smb2_check_message length: 0x48, smb_buf_length: 0x44 [ 48.234585] fs/cifs/smb2misc.c: SMB2 len 72 [ 48.235063] fs/cifs/transport.c: cifs_sync_mid_result: cmd=2 mid=5 state=4 [ 48.235541] SendRcvNoRsp flags 64 rc 0 [ 48.236075] fs/cifs/connect.c: CIFS VFS: leaving cifs_mount (xid = 0) rc = -5 [ 48.236556] CIFS VFS: cifs_mount failed w/return code = -5
Any thoughts on what is the right fix for stable kernels? Mounting SMB3 shares works great on mainline (v4.15-rc5). It also works on 4.4.109 if I pass the sec=ntlmsspi option to the mount command (as opposed to the default: sec=ntlmssp). Please let me know if you need any other info.
Thank you!
Regards, Srivatsa
Hi,
"Srivatsa S. Bhat" srivatsa@csail.mit.edu writes:
Any thoughts on what is the right fix for stable kernels? Mounting SMB3 shares works great on mainline (v4.15-rc5). It also works on 4.4.109 if I pass the sec=ntlmsspi option to the mount command (as opposed to the default: sec=ntlmssp). Please let me know if you need any other info.
Make sure you have (in that order):
db3b5474f462 ("CIFS: Fix NULL pointer deref on SMB2_tcon() failure") fe83bebc0522 ("SMB: fix leak of validate negotiate info response buffer") a2d9daad1d2d ("SMB: fix validate negotiate info uninitialised memory use") 4587eee04e2a ("SMB3: Validate negotiate request must always be signed") a821df3f1af7 ("cifs: fix NULL deref in SMB2_read")
Does enabling CIFS_SMB311 changes anything?
I also suspect some things assume encryption patches are in.
Hi Aurélien,
On 1/19/18 5:23 AM, Aurélien Aptel wrote:
Hi,
"Srivatsa S. Bhat" srivatsa@csail.mit.edu writes:
Any thoughts on what is the right fix for stable kernels? Mounting SMB3 shares works great on mainline (v4.15-rc5). It also works on 4.4.109 if I pass the sec=ntlmsspi option to the mount command (as opposed to the default: sec=ntlmssp). Please let me know if you need any other info.
Make sure you have (in that order):
db3b5474f462 ("CIFS: Fix NULL pointer deref on SMB2_tcon() failure") fe83bebc0522 ("SMB: fix leak of validate negotiate info response buffer") a2d9daad1d2d ("SMB: fix validate negotiate info uninitialised memory use") 4587eee04e2a ("SMB3: Validate negotiate request must always be signed") a821df3f1af7 ("cifs: fix NULL deref in SMB2_read")
Does enabling CIFS_SMB311 changes anything?
Thank you for looking into this. I tried applying these patches on top of 4.4.113 and 4.9.78, but that didn't fix the problem on either kernel, with or without CONFIG_CIFS_SMB311 enabled.
(By the way, shouldn't these patches be applied to stable kernels anyway? I was a bit surprised that none of them are present in 4.4.113 and 4.9.78).
I also suspect some things assume encryption patches are in.
Do you happen to know which patches they might be? In any case, I'm using the latest (unmodified) 4.4 and 4.9 stable kernels, so I hope the necessary support is already present in them.
The 5 patches you suggested above needed a bit of fixup by hand for 4.4.113, so I have shared my combined patch below for reference, which applies cleanly on top of 4.4.113. (The same patch applies on 4.9.78 as well, with some minor line-number differences).
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index f2ff60e..92abb8b9 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -519,7 +519,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) { int rc = 0; struct validate_negotiate_info_req vneg_inbuf; - struct validate_negotiate_info_rsp *pneg_rsp; + struct validate_negotiate_info_rsp *pneg_rsp = NULL; u32 rsplen;
cifs_dbg(FYI, "validate negotiate\n"); @@ -575,8 +575,9 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) rsplen);
/* relax check since Mac returns max bufsize allowed on ioctl */ - if (rsplen > CIFSMaxBufSize) - return -EIO; + if ((rsplen > CIFSMaxBufSize) + || (rsplen < sizeof(struct validate_negotiate_info_rsp))) + goto err_rsp_free; }
/* check validate negotiate info response matches what we got earlier */ @@ -595,10 +596,13 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
/* validate negotiate successful */ cifs_dbg(FYI, "validate negotiate info successful\n"); + kfree(pneg_rsp); return 0;
vneg_out: cifs_dbg(VFS, "protocol revalidation - security settings mismatch\n"); +err_rsp_free: + kfree(pneg_rsp); return -EIO; }
@@ -1042,7 +1046,7 @@ tcon_exit: return rc;
tcon_error_exit: - if (rsp->hdr.Status == STATUS_BAD_NETWORK_NAME) { + if (rsp && rsp->hdr.Status == STATUS_BAD_NETWORK_NAME) { cifs_dbg(VFS, "BAD_NETWORK_NAME: %s\n", tree); } goto tcon_exit; @@ -1559,6 +1563,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, } else iov[0].iov_len = get_rfc1002_length(req) + 4;
+ /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) + req->hdr.Flags |= SMB2_FLAGS_SIGNED;
rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0); rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base; @@ -2159,23 +2166,22 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms,
rsp = (struct smb2_read_rsp *)iov[0].iov_base;
- if (rsp->hdr.Status == STATUS_END_OF_FILE) { + if (rc) { + if (rc != -ENODATA) { + cifs_stats_fail_inc(io_parms->tcon, SMB2_READ_HE); + cifs_dbg(VFS, "Send error in read = %d\n", rc); + } free_rsp_buf(resp_buftype, iov[0].iov_base); - return 0; + return rc == -ENODATA ? 0 : rc; }
- if (rc) { - cifs_stats_fail_inc(io_parms->tcon, SMB2_READ_HE); - cifs_dbg(VFS, "Send error in read = %d\n", rc); - } else { - *nbytes = le32_to_cpu(rsp->DataLength); - if ((*nbytes > CIFS_MAX_MSGSIZE) || - (*nbytes > io_parms->length)) { - cifs_dbg(FYI, "bad length %d for count %d\n", - *nbytes, io_parms->length); - rc = -EIO; - *nbytes = 0; - } + *nbytes = le32_to_cpu(rsp->DataLength); + if ((*nbytes > CIFS_MAX_MSGSIZE) || + (*nbytes > io_parms->length)) { + cifs_dbg(FYI, "bad length %d for count %d\n", + *nbytes, io_parms->length); + rc = -EIO; + *nbytes = 0; }
if (*buf) {
With this patch (and CONFIG_CIFS_SMB311 enabled), the 4.4.113 kernel crashes as shown below when I try:
# mount -vvv -t cifs -o vers=3.0,credentials=.smbcred //<ip_addr>/TestSMB/ testdir
[ 14.638907] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050 [ 14.638940] IP: [<ffffffff8139221a>] crypto_shash_setkey+0x1a/0xc0 [ 14.638964] PGD 0 [ 14.638972] Oops: 0000 [#1] SMP [ 14.638985] Modules linked in: arc4(E) ecb(E) md4(E) cifs(E) dns_resolver(E) vmw_vsock_vmci_transport(E) vsock(E) xt_conntrack(E) iptable_nat(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) nf_nat_ipv4(E) nf_nat(E) iptable_filter(E) ip_tables(E) xt_LOG(E) nf_conntrack(E) hid_generic(E) usbhid(E) hid(E) mousedev(E) crc32c_intel(E) jitterentropy_rng(E) hmac(E) sha256_ssse3(E) sha256_generic(E) uhci_hcd(E) drbg(E) ansi_cprng(E) aesni_intel(E) ehci_pci(E) aes_x86_64(E) glue_helper(E) ehci_hcd(E) lrw(E) gf128mul(E) usbcore(E) ablk_helper(E) psmouse(E) cryptd(E) vmw_balloon(E) evdev(E) intel_agp(E) vmw_vmci(E) usb_common(E) i2c_piix4(E) intel_gtt(E) nfit(E) battery(E) tpm_tis(E) tpm(E) ac(E) button(E) sch_fq_codel(E) autofs4(E) [ 14.639237] CPU: 0 PID: 841 Comm: mount.cifs Tainted: G E 4.4.113-fixes-smb311+ #33 [ 14.639263] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016 [ 14.639294] task: ffff8800ae811440 ti: ffff8800b9d4c000 task.ti: ffff8800b9d4c000 [ 14.639315] RIP: 0010:[<ffffffff8139221a>] [<ffffffff8139221a>] crypto_shash_setkey+0x1a/0xc0 [ 14.639343] RSP: 0018:ffff8800b9d4f9a8 EFLAGS: 00010282 [ 14.639358] RAX: ffff88013305d580 RBX: ffff8800ba2ed000 RCX: 00000000fffee93f [ 14.639379] RDX: 0000000000000010 RSI: ffff8800b9f58d18 RDI: 0000000000000000 [ 14.639399] RBP: ffff8800b9d4f9e0 R08: ffff8800b9d4fb64 R09: 0000000000000000 [ 14.639420] R10: 3036312e3130312e R11: 424d53747365545c R12: 0000000000000002 [ 14.639440] R13: 0000000000000000 R14: ffff8800b9f58d18 R15: 0000000000000010 [ 14.639461] FS: 00007f02bcb74740(0000) GS:ffff88013fc00000(0000) knlGS:0000000000000000 [ 14.639484] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 14.639501] CR2: 0000000000000050 CR3: 00000000ae9f8000 CR4: 0000000000160670 [ 14.639558] Stack: [ 14.639566] ffff8800b66789c0 ffff8800b9d4fa08 ffff8800ba2ed000 0000000000000002 [ 14.639592] ffff8800b9d4fac8 00000c0094000029 ffff8800ba2ed000 ffff8800b9d4fa50 [ 14.639618] ffffffffa02594f6 ffff8800b9d4fb70 ffff88013305d580 0000000000000002 [ 14.639644] Call Trace: [ 14.639669] [<ffffffffa02594f6>] smb3_calc_signature+0xb6/0x290 [cifs] [ 14.639699] [<ffffffffa0258bab>] smb2_sign_rqst+0x2b/0x40 [cifs] [ 14.639726] [<ffffffffa02599d1>] smb2_setup_request+0xd1/0x170 [cifs] [ 14.640347] [<ffffffffa0248be7>] SendReceive2+0xc7/0x450 [cifs] [ 14.640958] [<ffffffffa02461b5>] ? cifs_small_buf_get+0x15/0x30 [cifs] [ 14.641582] [<ffffffffa025b89f>] ? small_smb2_init+0xdf/0x200 [cifs] [ 14.642172] [<ffffffffa025d867>] SMB2_ioctl+0x147/0x310 [cifs] [ 14.642753] [<ffffffffa025db37>] smb3_validate_negotiate+0x107/0x2e0 [cifs] [ 14.643336] [<ffffffffa025b1eb>] SMB2_tcon+0x29b/0x510 [cifs] [ 14.643921] [<ffffffffa0230c5b>] cifs_get_tcon+0x1bb/0x560 [cifs] [ 14.644501] [<ffffffffa02335f0>] cifs_mount+0x690/0xde0 [cifs] [ 14.645061] [<ffffffffa021f6eb>] cifs_do_mount+0xcb/0x5a0 [cifs] [ 14.645618] [<ffffffff81196057>] ? alloc_pages_current+0x87/0x110 [ 14.646149] [<ffffffff811baa83>] mount_fs+0x33/0x160 [ 14.646663] [<ffffffff811d4b22>] vfs_kern_mount+0x62/0x100 [ 14.647163] [<ffffffff811d6edb>] do_mount+0x21b/0xd30 [ 14.647653] [<ffffffff81196057>] ? alloc_pages_current+0x87/0x110 [ 14.648128] [<ffffffff811d7d07>] SyS_mount+0x87/0xd0 [ 14.648591] [<ffffffff817db31b>] entry_SYSCALL_64_fastpath+0x18/0x93 [ 14.649047] Code: 89 e5 8b 12 e8 a8 cd 04 00 31 c0 5d c3 0f 1f 40 00 55 48 89 e5 41 57 41 56 41 55 41 54 49 89 fd 53 49 89 f6 41 89 d7 48 83 ec 10 <4c> 8b 67 50 41 8b 5c 24 2c 48 85 de 75 14 41 ff 54 24 e8 48 83 [ 14.650496] RIP [<ffffffff8139221a>] crypto_shash_setkey+0x1a/0xc0 [ 14.650953] RSP <ffff8800b9d4f9a8> [ 14.651397] CR2: 0000000000000050 [ 14.651861] ---[ end trace c98f651d4ccb0d7d ]---
Regards, Srivatsa
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
4.13-stable review patch. If anyone has any objections, please let me know.
From: Steve French smfrench@gmail.com
commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
According to MS-SMB2 3.2.55 validate_negotiate request must always be signed. Some Windows can fail the request if you send it unsigned
See kernel bugzilla bug 197311
Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> Signed-off-by: Steve French smfrench@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/cifs/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+)
--- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc } else iov[0].iov_len = get_rfc1002_length(req) + 4;
- /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
- if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;This one needs to be backported to all stable kernels as the commit that introduced the regression: ' 0603c96f3af50e2f9299fa410c224ab1d465e0f9 SMB: Validate negotiate (to protect against downgrade) even if signing off
is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
Oh wait, it breaks the builds on older kernels, that's why I didn't apply it :)
Can you provide me with a working backport?
Hi Steve,
Is there a version of this fix available for stable kernels?
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Thank you!
Regards, Srivatsa
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
4.13-stable review patch. If anyone has any objections, please let me know.
From: Steve French smfrench@gmail.com
commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
According to MS-SMB2 3.2.55 validate_negotiate request must always be signed. Some Windows can fail the request if you send it unsigned
See kernel bugzilla bug 197311
Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> Signed-off-by: Steve French smfrench@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/cifs/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+)
--- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc } else iov[0].iov_len = get_rfc1002_length(req) + 4;
- /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
- if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;This one needs to be backported to all stable kernels as the commit that introduced the regression: ' 0603c96f3af50e2f9299fa410c224ab1d465e0f9 SMB: Validate negotiate (to protect against downgrade) even if signing off
is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
Oh wait, it breaks the builds on older kernels, that's why I didn't apply it :)
Can you provide me with a working backport?
Hi Steve,
Is there a version of this fix available for stable kernels?
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
thanks,
greg k-h
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:
4.13-stable review patch. If anyone has any objections, please let me know.
From: Steve French smfrench@gmail.com
commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream.
According to MS-SMB2 3.2.55 validate_negotiate request must always be signed. Some Windows can fail the request if you send it unsigned
See kernel bugzilla bug 197311
Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> Signed-off-by: Steve French smfrench@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
fs/cifs/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+)
--- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc } else iov[0].iov_len = get_rfc1002_length(req) + 4;
- /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
- if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;This one needs to be backported to all stable kernels as the commit that introduced the regression: ' 0603c96f3af50e2f9299fa410c224ab1d465e0f9 SMB: Validate negotiate (to protect against downgrade) even if signing off
is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
Oh wait, it breaks the builds on older kernels, that's why I didn't apply it :)
Can you provide me with a working backport?
Hi Steve,
Is there a version of this fix available for stable kernels?
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Thank you!
Regards, Srivatsa
On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote:
Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman: > 4.13-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Steve French smfrench@gmail.com > > commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream. > > According to MS-SMB2 3.2.55 validate_negotiate request must > always be signed. Some Windows can fail the request if you send it unsigned > > See kernel bugzilla bug 197311 > > Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> > Signed-off-by: Steve French smfrench@gmail.com > Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org > > --- > fs/cifs/smb2pdu.c | 3 +++ > 1 file changed, 3 insertions(+) > > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc > } else > iov[0].iov_len = get_rfc1002_length(req) + 4; > + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ > + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) > + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED; > rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov); > cifs_small_buf_release(req); > > >
This one needs to be backported to all stable kernels as the commit that introduced the regression: ' 0603c96f3af50e2f9299fa410c224ab1d465e0f9 SMB: Validate negotiate (to protect against downgrade) even if signing off
is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
Oh wait, it breaks the builds on older kernels, that's why I didn't apply it :)
Can you provide me with a working backport?
Hi Steve,
Is there a version of this fix available for stable kernels?
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Then what is the bugfix that should be applied here in order to keep things working with these patches applied?
thanks,
greg k-h
On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote:
On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote: > Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman: >> 4.13-stable review patch. If anyone has any objections, please let me know. >> >> ------------------ >> >> From: Steve French smfrench@gmail.com >> >> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream. >> >> According to MS-SMB2 3.2.55 validate_negotiate request must >> always be signed. Some Windows can fail the request if you send it unsigned >> >> See kernel bugzilla bug 197311 >> >> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> >> Signed-off-by: Steve French smfrench@gmail.com >> Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org >> >> --- >> fs/cifs/smb2pdu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> --- a/fs/cifs/smb2pdu.c >> +++ b/fs/cifs/smb2pdu.c >> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc >> } else >> iov[0].iov_len = get_rfc1002_length(req) + 4; >> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ >> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) >> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED; >> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov); >> cifs_small_buf_release(req); >> >> >> > > This one needs to be backported to all stable kernels as the commit that > introduced the regression: > ' > 0603c96f3af50e2f9299fa410c224ab1d465e0f9 > SMB: Validate negotiate (to protect against downgrade) even if signing off > > is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73
Oh wait, it breaks the builds on older kernels, that's why I didn't apply it :)
Can you provide me with a working backport?
Hi Steve,
Is there a version of this fix available for stable kernels?
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Then what is the bugfix that should be applied here in order to keep things working with these patches applied?
That would be the one mentioned in the subject line of this thread :) However, a working backport of that fix is not available for 4.4 and 4.9, hence the trouble.
It looks like we are reconstructing elements of this email thread all over again, so let me quickly summarize the status so far:
In 4.14/4.15/mainline, - commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against downgrade) even if signing off) caused mount regression with SMB v3.
- commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must always be signed) fixed the issue.
- [ There was a lot of code churn in the CIFS/SMB codebase between these two commits in mainline. ]
In this email thread, you backported the fix to stable 4.13. Thomas noticed that the problematic commit had also made it to stable series such as 4.4 and 4.9, and requested a backport of the fix to those trees as well. However, a straight-forward backport of the fix to 4.4 and 4.9 breaks the build, so no fix was available for those kernels.
I investigated this and tried to produce a working backport of the fix to 4.4 and 4.9, but didn't succeed, despite trying several variations as well as suggestions from Aurelien [1][2]. So, given that there is still no known fix for the mount regression on 4.4 and 4.9 stable series at this point, I decided to request a revert of the problematic commit that caused the regression in those kernels.
[1]. https://lkml.org/lkml/2018/1/3/892 [2]. https://lkml.org/lkml/2018/1/29/1009
Regards, Srivatsa
This shouldn't be too hard to figure out if willing to backport a slightly larger set of fixes to the older stable, but I don't have a system running 4.9 stable.
Is this the correct stable tree branch? https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/...
On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat srivatsa@csail.mit.edu wrote:
On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote:
On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote: > On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote: >> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman: >>> 4.13-stable review patch. If anyone has any objections, please let me know. >>> >>> ------------------ >>> >>> From: Steve French smfrench@gmail.com >>> >>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream. >>> >>> According to MS-SMB2 3.2.55 validate_negotiate request must >>> always be signed. Some Windows can fail the request if you send it unsigned >>> >>> See kernel bugzilla bug 197311 >>> >>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> >>> Signed-off-by: Steve French smfrench@gmail.com >>> Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org >>> >>> --- >>> fs/cifs/smb2pdu.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> --- a/fs/cifs/smb2pdu.c >>> +++ b/fs/cifs/smb2pdu.c >>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc >>> } else >>> iov[0].iov_len = get_rfc1002_length(req) + 4; >>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ >>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) >>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED; >>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov); >>> cifs_small_buf_release(req); >>> >>> >>> >> >> This one needs to be backported to all stable kernels as the commit that >> introduced the regression: >> ' >> 0603c96f3af50e2f9299fa410c224ab1d465e0f9 >> SMB: Validate negotiate (to protect against downgrade) even if signing off >> >> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73 > > Oh wait, it breaks the builds on older kernels, that's why I didn't > apply it :) > > Can you provide me with a working backport? >
Hi Steve,
Is there a version of this fix available for stable kernels?
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Then what is the bugfix that should be applied here in order to keep things working with these patches applied?
That would be the one mentioned in the subject line of this thread :) However, a working backport of that fix is not available for 4.4 and 4.9, hence the trouble.
It looks like we are reconstructing elements of this email thread all over again, so let me quickly summarize the status so far:
In 4.14/4.15/mainline,
commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against downgrade) even if signing off) caused mount regression with SMB v3.
commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must always be signed) fixed the issue.
[ There was a lot of code churn in the CIFS/SMB codebase between these two commits in mainline. ]
In this email thread, you backported the fix to stable 4.13. Thomas noticed that the problematic commit had also made it to stable series such as 4.4 and 4.9, and requested a backport of the fix to those trees as well. However, a straight-forward backport of the fix to 4.4 and 4.9 breaks the build, so no fix was available for those kernels.
I investigated this and tried to produce a working backport of the fix to 4.4 and 4.9, but didn't succeed, despite trying several variations as well as suggestions from Aurelien [1][2]. So, given that there is still no known fix for the mount regression on 4.4 and 4.9 stable series at this point, I decided to request a revert of the problematic commit that caused the regression in those kernels.
[1]. https://lkml.org/lkml/2018/1/3/892 [2]. https://lkml.org/lkml/2018/1/29/1009
Regards, Srivatsa
On 2/27/18 9:56 AM, Steve French wrote:
This shouldn't be too hard to figure out if willing to backport a slightly larger set of fixes to the older stable, but I don't have a system running 4.9 stable.
If you have the proposed patches that apply on 4.9, I'd be happy to try them out!
[ I would have offered to backport the patches myself, but actually I already tried doing that with a larger set of patches from mainline (picking those commits between the regression and the fix that seemed relevant), but I felt quite out-of-depth trying to adapt them to 4.9 and 4.4, as I'm not that familiar with the internals of SMB/CIFS. ]
Is this the correct stable tree branch? https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/...
Yep!
Regards, Srivatsa
On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat srivatsa@csail.mit.edu wrote:
On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote: > On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote: >> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote: >>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman: >>>> 4.13-stable review patch. If anyone has any objections, please let me know. >>>> >>>> ------------------ >>>> >>>> From: Steve French smfrench@gmail.com >>>> >>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream. >>>> >>>> According to MS-SMB2 3.2.55 validate_negotiate request must >>>> always be signed. Some Windows can fail the request if you send it unsigned >>>> >>>> See kernel bugzilla bug 197311 >>>> >>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> >>>> Signed-off-by: Steve French smfrench@gmail.com >>>> Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org >>>> >>>> --- >>>> fs/cifs/smb2pdu.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> --- a/fs/cifs/smb2pdu.c >>>> +++ b/fs/cifs/smb2pdu.c >>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc >>>> } else >>>> iov[0].iov_len = get_rfc1002_length(req) + 4; >>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ >>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) >>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED; >>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov); >>>> cifs_small_buf_release(req); >>>> >>>> >>>> >>> >>> This one needs to be backported to all stable kernels as the commit that >>> introduced the regression: >>> ' >>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9 >>> SMB: Validate negotiate (to protect against downgrade) even if signing off >>> >>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73 >> >> Oh wait, it breaks the builds on older kernels, that's why I didn't >> apply it :) >> >> Can you provide me with a working backport? >> > > Hi Steve, > > Is there a version of this fix available for stable kernels? >
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Then what is the bugfix that should be applied here in order to keep things working with these patches applied?
That would be the one mentioned in the subject line of this thread :) However, a working backport of that fix is not available for 4.4 and 4.9, hence the trouble.
It looks like we are reconstructing elements of this email thread all over again, so let me quickly summarize the status so far:
In 4.14/4.15/mainline,
commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against downgrade) even if signing off) caused mount regression with SMB v3.
commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must always be signed) fixed the issue.
[ There was a lot of code churn in the CIFS/SMB codebase between these two commits in mainline. ]
In this email thread, you backported the fix to stable 4.13. Thomas noticed that the problematic commit had also made it to stable series such as 4.4 and 4.9, and requested a backport of the fix to those trees as well. However, a straight-forward backport of the fix to 4.4 and 4.9 breaks the build, so no fix was available for those kernels.
I investigated this and tried to produce a working backport of the fix to 4.4 and 4.9, but didn't succeed, despite trying several variations as well as suggestions from Aurelien [1][2]. So, given that there is still no known fix for the mount regression on 4.4 and 4.9 stable series at this point, I decided to request a revert of the problematic commit that caused the regression in those kernels.
[1]. https://lkml.org/lkml/2018/1/3/892 [2]. https://lkml.org/lkml/2018/1/29/1009
Regards, Srivatsa
Just got a wireshark trace - this is a fairly trivial issue (missing the validate negotiate must be signed patch) - I had some trouble getting this version of the kernel running (unrelated issue) and on systems with access to Windows 2016...
On Tue, Feb 27, 2018 at 10:33 AM, Srivatsa S. Bhat srivatsa@csail.mit.edu wrote:
On 2/27/18 9:56 AM, Steve French wrote:
This shouldn't be too hard to figure out if willing to backport a slightly larger set of fixes to the older stable, but I don't have a system running 4.9 stable.
If you have the proposed patches that apply on 4.9, I'd be happy to try them out!
[ I would have offered to backport the patches myself, but actually I already tried doing that with a larger set of patches from mainline (picking those commits between the regression and the fix that seemed relevant), but I felt quite out-of-depth trying to adapt them to 4.9 and 4.4, as I'm not that familiar with the internals of SMB/CIFS. ]
Is this the correct stable tree branch? https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/...
Yep!
Regards, Srivatsa
On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat srivatsa@csail.mit.edu wrote:
On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote: > On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote: >> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote: >>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote: >>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman: >>>>> 4.13-stable review patch. If anyone has any objections, please let me know. >>>>> >>>>> ------------------ >>>>> >>>>> From: Steve French smfrench@gmail.com >>>>> >>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream. >>>>> >>>>> According to MS-SMB2 3.2.55 validate_negotiate request must >>>>> always be signed. Some Windows can fail the request if you send it unsigned >>>>> >>>>> See kernel bugzilla bug 197311 >>>>> >>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> >>>>> Signed-off-by: Steve French smfrench@gmail.com >>>>> Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org >>>>> >>>>> --- >>>>> fs/cifs/smb2pdu.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> --- a/fs/cifs/smb2pdu.c >>>>> +++ b/fs/cifs/smb2pdu.c >>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc >>>>> } else >>>>> iov[0].iov_len = get_rfc1002_length(req) + 4; >>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ >>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) >>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED; >>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov); >>>>> cifs_small_buf_release(req); >>>>> >>>>> >>>>> >>>> >>>> This one needs to be backported to all stable kernels as the commit that >>>> introduced the regression: >>>> ' >>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9 >>>> SMB: Validate negotiate (to protect against downgrade) even if signing off >>>> >>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73 >>> >>> Oh wait, it breaks the builds on older kernels, that's why I didn't >>> apply it :) >>> >>> Can you provide me with a working backport? >>> >> >> Hi Steve, >> >> Is there a version of this fix available for stable kernels? >> > > Hi Greg, > > Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 > due to the issues that I have described in detail on this mail thread. > > Since there is no apparent fix for this bug on stable kernels, could > you please consider reverting the original commit that caused this > regression? > > That commit was intended to enhance security, which is probably why it > was backported to stable kernels in the first place; but instead it > ends up breaking basic functionality itself (mounting). So in the > absence of a proper fix, I don't see much of an option but to revert > that commit. > > So, please consider reverting the following: > > commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect > against downgrade) even if signing off" on 4.4.118 > > commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect > against downgrade) even if signing off" on 4.9.84 > > They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 > upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Then what is the bugfix that should be applied here in order to keep things working with these patches applied?
That would be the one mentioned in the subject line of this thread :) However, a working backport of that fix is not available for 4.4 and 4.9, hence the trouble.
It looks like we are reconstructing elements of this email thread all over again, so let me quickly summarize the status so far:
In 4.14/4.15/mainline,
commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against downgrade) even if signing off) caused mount regression with SMB v3.
commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must always be signed) fixed the issue.
[ There was a lot of code churn in the CIFS/SMB codebase between these two commits in mainline. ]
In this email thread, you backported the fix to stable 4.13. Thomas noticed that the problematic commit had also made it to stable series such as 4.4 and 4.9, and requested a backport of the fix to those trees as well. However, a straight-forward backport of the fix to 4.4 and 4.9 breaks the build, so no fix was available for those kernels.
I investigated this and tried to produce a working backport of the fix to 4.4 and 4.9, but didn't succeed, despite trying several variations as well as suggestions from Aurelien [1][2]. So, given that there is still no known fix for the mount regression on 4.4 and 4.9 stable series at this point, I decided to request a revert of the problematic commit that caused the regression in those kernels.
[1]. https://lkml.org/lkml/2018/1/3/892 [2]. https://lkml.org/lkml/2018/1/29/1009
Regards, Srivatsa
On Sun, Mar 11, 2018 at 07:37:55PM -0700, Steve French wrote:
Just got a wireshark trace - this is a fairly trivial issue (missing the validate negotiate must be signed patch) - I had some trouble getting this version of the kernel running (unrelated issue) and on systems with access to Windows 2016...
Ok, I have no idea what this means, or what I should do here because of it :(
Any hints for what to do with the stable tree?
totally confused,
greg k-h
There will be a fix needed to correct an oops in calc_signature, besides the easy patch (smb3 validate negotiate patch).
On Tue, Mar 13, 2018 at 4:21 AM, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Sun, Mar 11, 2018 at 07:37:55PM -0700, Steve French wrote:
Just got a wireshark trace - this is a fairly trivial issue (missing the validate negotiate must be signed patch) - I had some trouble getting this version of the kernel running (unrelated issue) and on systems with access to Windows 2016...
Ok, I have no idea what this means, or what I should do here because of it :(
Any hints for what to do with the stable tree?
totally confused,
greg k-h
On Tue, Mar 13, 2018 at 10:21:45AM -0500, Steve French wrote:
There will be a fix needed to correct an oops in calc_signature, besides the easy patch (smb3 validate negotiate patch).
Ok, I still have no idea how to parse this for a stable tree submission.
So can someone please just send me a simple "apply these git ids to tree X.X.y so we can fix the problem", otherwise I'm not going to do anything here as I'm really confused,
greg k-h
Found a patch which solves the dependency issue. In my testing (on 4.9, with Windows 2016, and also to Samba) as Pavel suggested this appears to fix the problem, but I will let Srivatsa confirm that it also fixes it for him. The two attached patches for 4.9 should work.
As an aside which may help some in testing stable true problems (as a point of comparison or alternative), I did a complete backport of all relevant CIFS/SMB3 patches (ie all patches to cifs.ko that are not dependent on a VFS changes or global kernel API changes) for kernels 4.9 through 4.15 https://github.com/smfrench/smb3-cifs-linux-stable-backports
The individual patches that were included (and in a distinct directory all cifs patches that were rejected due to global/VFS dependencies) are also checked in - https://github.com/smfrench/smb3-backported-patches.
Given the focus on security, these two git trees may be useful for those who want a cifs.ko which includes all security and functional improvements and fixes that more closely matches mainline cifs.ko
Srivatsa, Let us know if those two patches fix your issue as expected.
On Fri, Mar 16, 2018 at 8:32 AM, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Tue, Mar 13, 2018 at 10:21:45AM -0500, Steve French wrote:
There will be a fix needed to correct an oops in calc_signature, besides the easy patch (smb3 validate negotiate patch).
Ok, I still have no idea how to parse this for a stable tree submission.
So can someone please just send me a simple "apply these git ids to tree X.X.y so we can fix the problem", otherwise I'm not going to do anything here as I'm really confused,
greg k-h
On 3/21/18 7:02 PM, Steve French wrote:
Found a patch which solves the dependency issue. In my testing (on 4.9, with Windows 2016, and also to Samba) as Pavel suggested this appears to fix the problem, but I will let Srivatsa confirm that it also fixes it for him. The two attached patches for 4.9 should work.
Indeed, those two patches fix the problem for me on 4.9. Thanks a lot Steve, Pavel and Aurelien for all your efforts in fixing this!
I was also interested in getting this fixed on 4.4, so I modified the patches to apply on 4.4.88 and verified that they fix the mount failure. I have attached my patches for 4.4 with this mail.
Steve, Pavel, could you kindly double-check the second patch for 4.4, especially around the keygen_exit error path?
Thank you very much!
Regards, Srivatsa VMware Photon OS
On 3/21/18 10:12 PM, Srivatsa S. Bhat wrote:
On 3/21/18 7:02 PM, Steve French wrote:
Found a patch which solves the dependency issue. In my testing (on 4.9, with Windows 2016, and also to Samba) as Pavel suggested this appears to fix the problem, but I will let Srivatsa confirm that it also fixes it for him. The two attached patches for 4.9 should work.
Indeed, those two patches fix the problem for me on 4.9. Thanks a lot Steve, Pavel and Aurelien for all your efforts in fixing this!
I was also interested in getting this fixed on 4.4, so I modified the patches to apply on 4.4.88 and verified that they fix the mount
I meant to say 4.4.122 there (the latest stable 4.4 version at the moment).
Regards, Srivatsa VMware Photon OS
On Wed, Mar 21, 2018 at 10:15:51PM -0700, Srivatsa S. Bhat wrote:
On 3/21/18 10:12 PM, Srivatsa S. Bhat wrote:
On 3/21/18 7:02 PM, Steve French wrote:
Found a patch which solves the dependency issue. In my testing (on 4.9, with Windows 2016, and also to Samba) as Pavel suggested this appears to fix the problem, but I will let Srivatsa confirm that it also fixes it for him. The two attached patches for 4.9 should work.
Indeed, those two patches fix the problem for me on 4.9. Thanks a lot Steve, Pavel and Aurelien for all your efforts in fixing this!
I was also interested in getting this fixed on 4.4, so I modified the patches to apply on 4.4.88 and verified that they fix the mount
I meant to say 4.4.122 there (the latest stable 4.4 version at the moment).
Thanks for these, all now queued up.
greg k-h
2018-03-21 22:12 GMT-07:00 Srivatsa S. Bhat srivatsa@csail.mit.edu:
On 3/21/18 7:02 PM, Steve French wrote:
Found a patch which solves the dependency issue. In my testing (on 4.9, with Windows 2016, and also to Samba) as Pavel suggested this appears to fix the problem, but I will let Srivatsa confirm that it also fixes it for him. The two attached patches for 4.9 should work.
Indeed, those two patches fix the problem for me on 4.9. Thanks a lot Steve, Pavel and Aurelien for all your efforts in fixing this!
I was also interested in getting this fixed on 4.4, so I modified the patches to apply on 4.4.88 and verified that they fix the mount failure. I have attached my patches for 4.4 with this mail.
Steve, Pavel, could you kindly double-check the second patch for 4.4, especially around the keygen_exit error path?
The patch looks good. Thanks for the backport.
-- Best regards, Pavel Shilovsky
So far I haven't been able to reproduce this on the current 4.9 stable tree with vers=3.0 or with default (vers=1.0 for these older kernels).
On Tue, Feb 27, 2018 at 11:56 AM, Steve French smfrench@gmail.com wrote:
This shouldn't be too hard to figure out if willing to backport a slightly larger set of fixes to the older stable, but I don't have a system running 4.9 stable.
Is this the correct stable tree branch? https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/...
On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat srivatsa@csail.mit.edu wrote:
On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote:
On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote: > On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote: >> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote: >>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman: >>>> 4.13-stable review patch. If anyone has any objections, please let me know. >>>> >>>> ------------------ >>>> >>>> From: Steve French smfrench@gmail.com >>>> >>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream. >>>> >>>> According to MS-SMB2 3.2.55 validate_negotiate request must >>>> always be signed. Some Windows can fail the request if you send it unsigned >>>> >>>> See kernel bugzilla bug 197311 >>>> >>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> >>>> Signed-off-by: Steve French smfrench@gmail.com >>>> Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org >>>> >>>> --- >>>> fs/cifs/smb2pdu.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> --- a/fs/cifs/smb2pdu.c >>>> +++ b/fs/cifs/smb2pdu.c >>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc >>>> } else >>>> iov[0].iov_len = get_rfc1002_length(req) + 4; >>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ >>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) >>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED; >>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov); >>>> cifs_small_buf_release(req); >>>> >>>> >>>> >>> >>> This one needs to be backported to all stable kernels as the commit that >>> introduced the regression: >>> ' >>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9 >>> SMB: Validate negotiate (to protect against downgrade) even if signing off >>> >>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73 >> >> Oh wait, it breaks the builds on older kernels, that's why I didn't >> apply it :) >> >> Can you provide me with a working backport? >> > > Hi Steve, > > Is there a version of this fix available for stable kernels? >
Hi Greg,
Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 due to the issues that I have described in detail on this mail thread.
Since there is no apparent fix for this bug on stable kernels, could you please consider reverting the original commit that caused this regression?
That commit was intended to enhance security, which is probably why it was backported to stable kernels in the first place; but instead it ends up breaking basic functionality itself (mounting). So in the absence of a proper fix, I don't see much of an option but to revert that commit.
So, please consider reverting the following:
commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.4.118
commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect against downgrade) even if signing off" on 4.9.84
They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Then what is the bugfix that should be applied here in order to keep things working with these patches applied?
That would be the one mentioned in the subject line of this thread :) However, a working backport of that fix is not available for 4.4 and 4.9, hence the trouble.
It looks like we are reconstructing elements of this email thread all over again, so let me quickly summarize the status so far:
In 4.14/4.15/mainline,
commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against downgrade) even if signing off) caused mount regression with SMB v3.
commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must always be signed) fixed the issue.
[ There was a lot of code churn in the CIFS/SMB codebase between these two commits in mainline. ]
In this email thread, you backported the fix to stable 4.13. Thomas noticed that the problematic commit had also made it to stable series such as 4.4 and 4.9, and requested a backport of the fix to those trees as well. However, a straight-forward backport of the fix to 4.4 and 4.9 breaks the build, so no fix was available for those kernels.
I investigated this and tried to produce a working backport of the fix to 4.4 and 4.9, but didn't succeed, despite trying several variations as well as suggestions from Aurelien [1][2]. So, given that there is still no known fix for the mount regression on 4.4 and 4.9 stable series at this point, I decided to request a revert of the problematic commit that caused the regression in those kernels.
[1]. https://lkml.org/lkml/2018/1/3/892 [2]. https://lkml.org/lkml/2018/1/29/1009
Regards, Srivatsa
-- Thanks,
Steve
On 3/1/18 12:12 PM, Steve French wrote:
So far I haven't been able to reproduce this on the current 4.9 stable tree with vers=3.0 or with default (vers=1.0 for these older kernels).
Maybe the problem also depends on the particular version of Windows that hosts the SMB shares? I'm using Windows Server 2016 (Version 1607, OS Build 14393.693). With vers=3.0, the issue is reproducible every time, but vers=1.0 works fine.
Regards, Srivatsa
On Tue, Feb 27, 2018 at 11:56 AM, Steve French smfrench@gmail.com wrote:
This shouldn't be too hard to figure out if willing to backport a slightly larger set of fixes to the older stable, but I don't have a system running 4.9 stable.
Is this the correct stable tree branch? https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/...
On Tue, Feb 27, 2018 at 11:45 AM, Srivatsa S. Bhat srivatsa@csail.mit.edu wrote:
On 2/27/18 4:40 AM, Greg Kroah-Hartman wrote:
On Tue, Feb 27, 2018 at 01:22:31AM -0800, Srivatsa S. Bhat wrote:
On 2/27/18 12:54 AM, Greg Kroah-Hartman wrote:
On Mon, Feb 26, 2018 at 07:44:28PM -0800, Srivatsa S. Bhat wrote: > On 1/3/18 6:15 PM, Srivatsa S. Bhat wrote: >> On 11/1/17 8:18 AM, Greg Kroah-Hartman wrote: >>> On Tue, Oct 31, 2017 at 03:02:11PM +0200, Thomas Backlund wrote: >>>> Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman: >>>>> 4.13-stable review patch. If anyone has any objections, please let me know. >>>>> >>>>> ------------------ >>>>> >>>>> From: Steve French smfrench@gmail.com >>>>> >>>>> commit 4587eee04e2ac7ac3ac9fa2bc164fb6e548f99cd upstream. >>>>> >>>>> According to MS-SMB2 3.2.55 validate_negotiate request must >>>>> always be signed. Some Windows can fail the request if you send it unsigned >>>>> >>>>> See kernel bugzilla bug 197311 >>>>> >>>>> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> >>>>> Signed-off-by: Steve French smfrench@gmail.com >>>>> Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org >>>>> >>>>> --- >>>>> fs/cifs/smb2pdu.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> --- a/fs/cifs/smb2pdu.c >>>>> +++ b/fs/cifs/smb2pdu.c >>>>> @@ -1963,6 +1963,9 @@ SMB2_ioctl(const unsigned int xid, struc >>>>> } else >>>>> iov[0].iov_len = get_rfc1002_length(req) + 4; >>>>> + /* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */ >>>>> + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) >>>>> + req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED; >>>>> rc = SendReceive2(xid, ses, iov, n_iov, &resp_buftype, flags, &rsp_iov); >>>>> cifs_small_buf_release(req); >>>>> >>>>> >>>>> >>>> >>>> This one needs to be backported to all stable kernels as the commit that >>>> introduced the regression: >>>> ' >>>> 0603c96f3af50e2f9299fa410c224ab1d465e0f9 >>>> SMB: Validate negotiate (to protect against downgrade) even if signing off >>>> >>>> is backported in stable trees as of: 4.9.53, 4.4.90, 3.18.73 >>> >>> Oh wait, it breaks the builds on older kernels, that's why I didn't >>> apply it :) >>> >>> Can you provide me with a working backport? >>> >> >> Hi Steve, >> >> Is there a version of this fix available for stable kernels? >> > > Hi Greg, > > Mounting SMB3 shares continues to fail for me on 4.4.118 and 4.9.84 > due to the issues that I have described in detail on this mail thread. > > Since there is no apparent fix for this bug on stable kernels, could > you please consider reverting the original commit that caused this > regression? > > That commit was intended to enhance security, which is probably why it > was backported to stable kernels in the first place; but instead it > ends up breaking basic functionality itself (mounting). So in the > absence of a proper fix, I don't see much of an option but to revert > that commit. > > So, please consider reverting the following: > > commit 02ef29f9cbb616bf419 "SMB: Validate negotiate (to protect > against downgrade) even if signing off" on 4.4.118 > > commit 0e1b85a41a25ac888fb "SMB: Validate negotiate (to protect > against downgrade) even if signing off" on 4.9.84 > > They correspond to commit 0603c96f3af50e2f9299fa410c224ab1d465e0f9 > upstream. Both these patches should revert cleanly.
Do you still have this same problem on 4.14 and 4.15? If so, the issue needs to get fixed there, not papered-over by reverting these old changes, as you will hit the issue again in the future when you update to a newer kernel version.
4.14 and 4.15 work great! (I had mentioned this is in my original bug report but forgot to summarize it here, sorry).
Then what is the bugfix that should be applied here in order to keep things working with these patches applied?
That would be the one mentioned in the subject line of this thread :) However, a working backport of that fix is not available for 4.4 and 4.9, hence the trouble.
It looks like we are reconstructing elements of this email thread all over again, so let me quickly summarize the status so far:
In 4.14/4.15/mainline,
commit 0603c96f3af50e2f9 (SMB: Validate negotiate (to protect against downgrade) even if signing off) caused mount regression with SMB v3.
commit 4587eee04e2ac7ac3 (SMB3: Validate negotiate request must always be signed) fixed the issue.
[ There was a lot of code churn in the CIFS/SMB codebase between these two commits in mainline. ]
In this email thread, you backported the fix to stable 4.13. Thomas noticed that the problematic commit had also made it to stable series such as 4.4 and 4.9, and requested a backport of the fix to those trees as well. However, a straight-forward backport of the fix to 4.4 and 4.9 breaks the build, so no fix was available for those kernels.
I investigated this and tried to produce a working backport of the fix to 4.4 and 4.9, but didn't succeed, despite trying several variations as well as suggestions from Aurelien [1][2]. So, given that there is still no known fix for the mount regression on 4.4 and 4.9 stable series at this point, I decided to request a revert of the problematic commit that caused the regression in those kernels.
[1]. https://lkml.org/lkml/2018/1/3/892 [2]. https://lkml.org/lkml/2018/1/29/1009
Regards, Srivatsa
-- Thanks,
Steve
linux-stable-mirror@lists.linaro.org
-
Aurélien Aptel -
Greg Kroah-Hartman -
Pavel Shilovsky -
Srivatsa S. Bhat -
Steve French