4.13.11-stable review

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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?

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 2 lines

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:

fs/cifs/smb2pdu.c +3 -0

index f2ff60e..3963bd2 100644
@@ -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:

fs/cifs/smb2pdu.c +4 -1

index f2ff60e..19cc92c 100644
@@ -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

Quoted 4 lines

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.

Hi,

Quoted 4 lines · Srivatsa S. Bhat

Quoted 4 lines

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.

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 2 lines

Can you provide me with a working backport?

Hi Steve,

Is there a version of this fix available for stable kernels?

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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.

Quoted 81 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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

Quoted 2 lines

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.

Quoted 85 lines · Greg Kroah-Hartman

Quoted 83 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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.

Quoted 5 lines

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

Quoted 92 lines · Srivatsa S. Bhat

Quoted 90 lines · Greg Kroah-Hartman

Quoted 75 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 2 lines

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?

Quoted 8 lines · Greg Kroah-Hartman

Quoted 7 lines · Srivatsa S. Bhat

Quoted 6 lines · Greg Kroah-Hartman

Quoted 5 lines · Srivatsa S. Bhat

Quoted 4 lines · Srivatsa S. Bhat

Quoted 3 lines · Greg Kroah-Hartman

Quoted 2 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 1 line

4.13-stable review patch. If anyone has any objections, please

let me know.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

------------------

From: Steve French <[email protected]>

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

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

See kernel bugzilla bug 197311

Acked-by Ronnie Sahlberg <lsahlber.redhat.com>

Signed-off-by Steve French <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1965

Quoted 6 lines

fs/cifs/smb2pdu.c +1 -0

@@ -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 */

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1968

Quoted 3 lines

fs/cifs/smb2pdu.c +2 -0

+      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);

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 4 lines

fs/cifs/smb2pdu.c +0 -0

       cifs_small_buf_release(req);
 
 

This one needs to be backported to all stable kernels as the

commit that

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

introduced the regression:
'
0603c96f3af50e2f9299fa410c224ab1d465e0f9
SMB: Validate negotiate (to protect against downgrade) even if

signing off

Quoted 46 lines

Quoted 46 lines

Quoted 45 lines

Quoted 31 lines

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

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

Quoted 7 lines

Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 7 lines

Quoted 2 lines

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

Quoted 93 lines · Greg Kroah-Hartman

Quoted 92 lines · Srivatsa S. Bhat

Quoted 90 lines · Greg Kroah-Hartman

Quoted 75 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 7 lines

Quoted 2 lines

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/?h=linux-4.9.y

Quoted 137 lines · Srivatsa S. Bhat

Quoted 93 lines · Greg Kroah-Hartman

Quoted 92 lines · Srivatsa S. Bhat

Quoted 90 lines · Greg Kroah-Hartman

Quoted 75 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 7 lines

Quoted 2 lines

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

Quoted 4 lines · Steve French

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

Quoted 3 lines

Is this the correct stable tree branch?
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-4.9.y

Yep!

Regards,
Srivatsa

Quoted 142 lines

Quoted 137 lines · Srivatsa S. Bhat

Quoted 93 lines · Greg Kroah-Hartman

Quoted 92 lines · Srivatsa S. Bhat

Quoted 90 lines · Greg Kroah-Hartman

Quoted 75 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 7 lines

Quoted 2 lines

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

Quoted 4 lines · Steve French

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

Quoted 2 lines · Steve French

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

Quoted 2 lines · Steve French

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.

Quoted 5 lines · Steve French

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

Quoted 7 lines · Srivatsa S. Bhat

Quoted 5 lines · Steve French

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.

Quoted 5 lines

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

Quoted 5 lines

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?

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/?h=linux-4.9.y

Quoted 132 lines · Srivatsa S. Bhat

Quoted 93 lines · Greg Kroah-Hartman

Quoted 92 lines · Srivatsa S. Bhat

Quoted 90 lines · Greg Kroah-Hartman

Quoted 75 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 7 lines

Quoted 2 lines

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.

Quoted 5 lines

[1]. https://lkml.org/lkml/2018/1/3/892
[2]. https://lkml.org/lkml/2018/1/29/1009

Regards,
Srivatsa

--
Thanks,

Steve

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

Quoted 153 lines

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/?h=linux-4.9.y

Quoted 132 lines · Srivatsa S. Bhat

Quoted 93 lines · Greg Kroah-Hartman

Quoted 92 lines · Srivatsa S. Bhat

Quoted 90 lines · Greg Kroah-Hartman

Quoted 75 lines · Srivatsa S. Bhat

Quoted 49 lines · Srivatsa S. Bhat

Quoted 48 lines · Greg Kroah-Hartman

Quoted 42 lines · Thomas Backlund

Den 31.10.2017 kl. 11:55, skrev Greg Kroah-Hartman:

Quoted 25 lines

4.13-stable review patch. If anyone has any objections, please let me know.

------------------

From: Steve French <[email protected]>

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 <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  fs/cifs/smb2pdu.c |    3 +++
  1 file changed, 3 insertions(+)

fs/cifs/smb2pdu.c SMB2_ioctl(const unsigned int xid, struc · line 1971

Quoted 9 lines

fs/cifs/smb2pdu.c +3 -0

@@ -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

Quoted 1 line

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

Quoted 7 lines

Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 7 lines

Quoted 2 lines

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.

Quoted 5 lines

[1]. https://lkml.org/lkml/2018/1/3/892
[2]. https://lkml.org/lkml/2018/1/29/1009

Regards,
Srivatsa

--
Thanks,

Steve