* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed [not found] ` <20171101151803.GB31285@kroah.com> @ 2018-01-04 2:15 ` Srivatsa S. Bhat 2018-01-18 21:25 ` Srivatsa S. Bhat 2018-02-27 3:44 ` Srivatsa S. Bhat 0 siblings, 2 replies; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-01-04 2:15 UTC (permalink / raw) To: Greg Kroah-Hartman, Thomas Backlund, Steve French Cc: linux-kernel, stable, lsahlber, pshilov, linux-cifs 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? 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-01-04 2:15 ` [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed Srivatsa S. Bhat @ 2018-01-18 21:25 ` Srivatsa S. Bhat 2018-01-19 13:23 ` Aurélien Aptel 2018-02-27 3:44 ` Srivatsa S. Bhat 1 sibling, 1 reply; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-01-18 21:25 UTC (permalink / raw) To: Greg Kroah-Hartman, Thomas Backlund, Steve French Cc: linux-kernel, stable, lsahlber, pshilov, linux-cifs 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? > 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 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-01-18 21:25 ` Srivatsa S. Bhat @ 2018-01-19 13:23 ` Aurélien Aptel [not found] ` <87lggux9rp.fsf-IBi9RG/b67k@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Aurélien Aptel @ 2018-01-19 13:23 UTC (permalink / raw) To: Srivatsa S. Bhat, Greg Kroah-Hartman, Thomas Backlund, Steve French Cc: linux-kernel, stable, lsahlber, pshilov, linux-cifs 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. -- Aurélien Aptel / SUSE Labs Samba Team GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <87lggux9rp.fsf-IBi9RG/b67k@public.gmane.org>]
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed [not found] ` <87lggux9rp.fsf-IBi9RG/b67k@public.gmane.org> @ 2018-01-30 3:31 ` Srivatsa S. Bhat 0 siblings, 0 replies; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-01-30 3:31 UTC (permalink / raw) To: Aurélien Aptel, Greg Kroah-Hartman, Thomas Backlund, Steve French Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, stable-u79uwXL29TY76Z2rM5mHXA, lsahlber-H+wXaHxf7aLQT0dZR+AlfA, pshilov-0li6OtcxBFHby3iVrkZq2A, linux-cifs-u79uwXL29TY76Z2rM5mHXA Hi Aurélien, On 1/19/18 5:23 AM, Aurélien Aptel wrote: > Hi, > > "Srivatsa S. Bhat" <srivatsa-EfZU8u+QLuPpgkiH4x7ZXw@public.gmane.org> 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-01-04 2:15 ` [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed Srivatsa S. Bhat 2018-01-18 21:25 ` Srivatsa S. Bhat @ 2018-02-27 3:44 ` Srivatsa S. Bhat 2018-02-27 8:54 ` Greg Kroah-Hartman 1 sibling, 1 reply; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-02-27 3:44 UTC (permalink / raw) To: Greg Kroah-Hartman, Thomas Backlund, Steve French, Aurélien Aptel Cc: linux-kernel, stable, lsahlber, pshilov, linux-cifs 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. Thank you! Regards, Srivatsa ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 3:44 ` Srivatsa S. Bhat @ 2018-02-27 8:54 ` Greg Kroah-Hartman 2018-02-27 9:22 ` Srivatsa S. Bhat 0 siblings, 1 reply; 22+ messages in thread From: Greg Kroah-Hartman @ 2018-02-27 8:54 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Thomas Backlund, Steve French, Aurélien Aptel, linux-kernel, stable, lsahlber, pshilov, linux-cifs 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. thanks, greg k-h ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 8:54 ` Greg Kroah-Hartman @ 2018-02-27 9:22 ` Srivatsa S. Bhat 2018-02-27 12:40 ` Greg Kroah-Hartman 0 siblings, 1 reply; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-02-27 9:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Thomas Backlund, Steve French, Aurélien Aptel, linux-kernel, stable, lsahlber, pshilov, linux-cifs 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). Thank you! Regards, Srivatsa ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 9:22 ` Srivatsa S. Bhat @ 2018-02-27 12:40 ` Greg Kroah-Hartman 2018-02-27 17:45 ` Srivatsa S. Bhat 0 siblings, 1 reply; 22+ messages in thread From: Greg Kroah-Hartman @ 2018-02-27 12:40 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Thomas Backlund, Steve French, Aurélien Aptel, linux-kernel, stable, lsahlber, pshilov, linux-cifs 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 12:40 ` Greg Kroah-Hartman @ 2018-02-27 17:45 ` Srivatsa S. Bhat 2018-02-27 17:56 ` Steve French 0 siblings, 1 reply; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-02-27 17:45 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Thomas Backlund, Steve French, Aurélien Aptel, linux-kernel, stable, lsahlber, pshilov, linux-cifs 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 17:45 ` Srivatsa S. Bhat @ 2018-02-27 17:56 ` Steve French 2018-02-27 18:33 ` Srivatsa S. Bhat 2018-03-01 20:12 ` Steve French 0 siblings, 2 replies; 22+ messages in thread From: Steve French @ 2018-02-27 17:56 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Greg Kroah-Hartman, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 17:56 ` Steve French @ 2018-02-27 18:33 ` Srivatsa S. Bhat 2018-03-12 2:37 ` Steve French 2018-03-01 20:12 ` Steve French 1 sibling, 1 reply; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-02-27 18:33 UTC (permalink / raw) To: Steve French Cc: Greg Kroah-Hartman, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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/?h=linux-4.9.y > 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 > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 18:33 ` Srivatsa S. Bhat @ 2018-03-12 2:37 ` Steve French 2018-03-13 9:21 ` Greg Kroah-Hartman 0 siblings, 1 reply; 22+ messages in thread From: Steve French @ 2018-03-12 2:37 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Greg Kroah-Hartman, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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/?h=linux-4.9.y >> > > 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 >> >> >> > -- Thanks, Steve ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-12 2:37 ` Steve French @ 2018-03-13 9:21 ` Greg Kroah-Hartman 2018-03-13 15:21 ` Steve French 0 siblings, 1 reply; 22+ messages in thread From: Greg Kroah-Hartman @ 2018-03-13 9:21 UTC (permalink / raw) To: Steve French Cc: Srivatsa S. Bhat, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-13 9:21 ` Greg Kroah-Hartman @ 2018-03-13 15:21 ` Steve French 2018-03-16 13:32 ` Greg Kroah-Hartman 0 siblings, 1 reply; 22+ messages in thread From: Steve French @ 2018-03-13 15:21 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Srivatsa S. Bhat, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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 -- Thanks, Steve ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-13 15:21 ` Steve French @ 2018-03-16 13:32 ` Greg Kroah-Hartman 2018-03-22 2:02 ` Steve French 0 siblings, 1 reply; 22+ messages in thread From: Greg Kroah-Hartman @ 2018-03-16 13:32 UTC (permalink / raw) To: Steve French Cc: Srivatsa S. Bhat, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-16 13:32 ` Greg Kroah-Hartman @ 2018-03-22 2:02 ` Steve French 2018-03-22 5:12 ` Srivatsa S. Bhat 0 siblings, 1 reply; 22+ messages in thread From: Steve French @ 2018-03-22 2:02 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Srivatsa S. Bhat, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS [-- Attachment #1: Type: text/plain, Size: 1692 bytes --] 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 -- Thanks, Steve [-- Attachment #2: 0001-SMB3-Validate-negotiate-request-must-always-be-signe.patch --] [-- Type: text/x-patch, Size: 1223 bytes --] From 8ac7b1d15dc973e2092ab2b1b5b698eb92e1d1c3 Mon Sep 17 00:00:00 2001 From: Steve French <smfrench@gmail.com> Date: Sun, 11 Mar 2018 20:00:27 -0700 Subject: [PATCH 1/2] SMB3: Validate negotiate request must always be signed 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 [Patch fixed up for kernel version 4.9] CC: Stable <stable@vger.kernel.org> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> Signed-off-by: Steve French <smfrench@gmail.com> --- fs/cifs/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 94c4c1901222..4c2eaf05a6a4 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1712,6 +1712,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; -- 2.14.1 [-- Attachment #3: 0002-CIFS-Enable-encryption-during-session-setup-phase.patch --] [-- Type: text/x-patch, Size: 3310 bytes --] From c5346223ca952a2868bd69a8888133251e517571 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky <pshilov@microsoft.com> Date: Mon, 7 Nov 2016 18:20:50 -0800 Subject: [PATCH 2/2] CIFS: Enable encryption during session setup phase In order to allow encryption on SMB connection we need to exchange a session key and generate encryption and decryption keys. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> --- fs/cifs/sess.c | 22 ++++++++++------------ fs/cifs/smb2pdu.c | 12 ++---------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index 538d9b55699a..c3db2a882aee 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -344,13 +344,12 @@ void build_ntlmssp_negotiate_blob(unsigned char *pbuffer, /* BB is NTLMV2 session security format easier to use here? */ flags = NTLMSSP_NEGOTIATE_56 | NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE | - NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC; - if (ses->server->sign) { + NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC | + NTLMSSP_NEGOTIATE_SEAL; + if (ses->server->sign) flags |= NTLMSSP_NEGOTIATE_SIGN; - if (!ses->server->session_estab || - ses->ntlmssp->sesskey_per_smbsess) - flags |= NTLMSSP_NEGOTIATE_KEY_XCH; - } + if (!ses->server->session_estab || ses->ntlmssp->sesskey_per_smbsess) + flags |= NTLMSSP_NEGOTIATE_KEY_XCH; sec_blob->NegotiateFlags = cpu_to_le32(flags); @@ -407,13 +406,12 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer, flags = NTLMSSP_NEGOTIATE_56 | NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_TARGET_INFO | NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE | - NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC; - if (ses->server->sign) { + NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC | + NTLMSSP_NEGOTIATE_SEAL; + if (ses->server->sign) flags |= NTLMSSP_NEGOTIATE_SIGN; - if (!ses->server->session_estab || - ses->ntlmssp->sesskey_per_smbsess) - flags |= NTLMSSP_NEGOTIATE_KEY_XCH; - } + if (!ses->server->session_estab || ses->ntlmssp->sesskey_per_smbsess) + flags |= NTLMSSP_NEGOTIATE_KEY_XCH; tmp = *pbuffer + sizeof(AUTHENTICATE_MESSAGE); sec_blob->NegotiateFlags = cpu_to_le32(flags); diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 4c2eaf05a6a4..7c26286a525d 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -707,15 +707,13 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data) struct cifs_ses *ses = sess_data->ses; mutex_lock(&ses->server->srv_mutex); - if (ses->server->sign && ses->server->ops->generate_signingkey) { + if (ses->server->ops->generate_signingkey) { rc = ses->server->ops->generate_signingkey(ses); - kfree(ses->auth_key.response); - ses->auth_key.response = NULL; if (rc) { cifs_dbg(FYI, "SMB3 session key generation failed\n"); mutex_unlock(&ses->server->srv_mutex); - goto keygen_exit; + return rc; } } if (!ses->server->session_estab) { @@ -729,12 +727,6 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data) ses->status = CifsGood; ses->need_reconnect = false; spin_unlock(&GlobalMid_Lock); - -keygen_exit: - if (!ses->server->sign) { - kfree(ses->auth_key.response); - ses->auth_key.response = NULL; - } return rc; } -- 2.14.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-22 2:02 ` Steve French @ 2018-03-22 5:12 ` Srivatsa S. Bhat 2018-03-22 5:15 ` Srivatsa S. Bhat 2018-03-22 19:14 ` Pavel Shilovsky 0 siblings, 2 replies; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-03-22 5:12 UTC (permalink / raw) To: Steve French, Greg Kroah-Hartman Cc: Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS [-- Attachment #1: Type: text/plain, Size: 828 bytes --] 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 [-- Attachment #2: v4.4-0001-SMB3-Validate-negotiate-request-must-always-be-signe.patch --] [-- Type: text/plain, Size: 1324 bytes --] From a01a7dfb60e2d5421a487a7b81fd8a1bf72d96d4 Mon Sep 17 00:00:00 2001 From: Steve French <smfrench@gmail.com> Date: Sun, 11 Mar 2018 20:00:27 -0700 Subject: [PATCH 1/2] SMB3: Validate negotiate request must always be signed 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 [ Fixed up for kernel version 4.4 ] CC: Stable <stable@vger.kernel.org> Acked-by: Ronnie Sahlberg <lsahlber.redhat.com> Signed-off-by: Steve French <smfrench@gmail.com> Signed-off-by: Srivatsa S. Bhat <srivatsa@csail.mit.edu> --- fs/cifs/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 84614a5..6dae5b8 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1558,6 +1558,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; -- 2.7.4 [-- Attachment #3: v4.4-0002-CIFS-Enable-encryption-during-session-setup-phase.patch --] [-- Type: text/plain, Size: 3136 bytes --] From d0178d8f096b29a88914787274bdc8ee8334ab07 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky <pshilov@microsoft.com> Date: Mon, 7 Nov 2016 18:20:50 -0800 Subject: [PATCH 2/2] CIFS: Enable encryption during session setup phase commit cabfb3680f78981d26c078a26e5c748531257ebb upstream. In order to allow encryption on SMB connection we need to exchange a session key and generate encryption and decryption keys. [ Fixed up for kernel version 4.4 ] Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Srivatsa S. Bhat <srivatsa@csail.mit.edu> --- fs/cifs/sess.c | 22 ++++++++++------------ fs/cifs/smb2pdu.c | 8 +------- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index e88ffe1..a035d1a 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -344,13 +344,12 @@ void build_ntlmssp_negotiate_blob(unsigned char *pbuffer, /* BB is NTLMV2 session security format easier to use here? */ flags = NTLMSSP_NEGOTIATE_56 | NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE | - NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC; - if (ses->server->sign) { + NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC | + NTLMSSP_NEGOTIATE_SEAL; + if (ses->server->sign) flags |= NTLMSSP_NEGOTIATE_SIGN; - if (!ses->server->session_estab || - ses->ntlmssp->sesskey_per_smbsess) - flags |= NTLMSSP_NEGOTIATE_KEY_XCH; - } + if (!ses->server->session_estab || ses->ntlmssp->sesskey_per_smbsess) + flags |= NTLMSSP_NEGOTIATE_KEY_XCH; sec_blob->NegotiateFlags = cpu_to_le32(flags); @@ -407,13 +406,12 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer, flags = NTLMSSP_NEGOTIATE_56 | NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_TARGET_INFO | NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE | - NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC; - if (ses->server->sign) { + NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC | + NTLMSSP_NEGOTIATE_SEAL; + if (ses->server->sign) flags |= NTLMSSP_NEGOTIATE_SIGN; - if (!ses->server->session_estab || - ses->ntlmssp->sesskey_per_smbsess) - flags |= NTLMSSP_NEGOTIATE_KEY_XCH; - } + if (!ses->server->session_estab || ses->ntlmssp->sesskey_per_smbsess) + flags |= NTLMSSP_NEGOTIATE_KEY_XCH; tmp = *pbuffer + sizeof(AUTHENTICATE_MESSAGE); sec_blob->NegotiateFlags = cpu_to_le32(flags); diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 6dae5b8..33b1bc2 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -832,10 +832,8 @@ 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; if (rc) { cifs_dbg(FYI, "SMB3 session key generation failed\n"); @@ -857,10 +855,6 @@ ssetup_exit: } keygen_exit: - if (!server->sign) { - kfree(ses->auth_key.response); - ses->auth_key.response = NULL; - } if (spnego_key) { key_invalidate(spnego_key); key_put(spnego_key); -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-22 5:12 ` Srivatsa S. Bhat @ 2018-03-22 5:15 ` Srivatsa S. Bhat 2018-03-22 10:32 ` Greg Kroah-Hartman 2018-03-22 19:14 ` Pavel Shilovsky 1 sibling, 1 reply; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-03-22 5:15 UTC (permalink / raw) To: Steve French, Greg Kroah-Hartman Cc: Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-22 5:15 ` Srivatsa S. Bhat @ 2018-03-22 10:32 ` Greg Kroah-Hartman 0 siblings, 0 replies; 22+ messages in thread From: Greg Kroah-Hartman @ 2018-03-22 10:32 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Steve French, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-22 5:12 ` Srivatsa S. Bhat 2018-03-22 5:15 ` Srivatsa S. Bhat @ 2018-03-22 19:14 ` Pavel Shilovsky 1 sibling, 0 replies; 22+ messages in thread From: Pavel Shilovsky @ 2018-03-22 19:14 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Steve French, Greg Kroah-Hartman, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-02-27 17:56 ` Steve French 2018-02-27 18:33 ` Srivatsa S. Bhat @ 2018-03-01 20:12 ` Steve French 2018-03-01 20:51 ` Srivatsa S. Bhat 1 sibling, 1 reply; 22+ messages in thread From: Steve French @ 2018-03-01 20:12 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Greg Kroah-Hartman, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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/?h=linux-4.9.y > > 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 -- Thanks, Steve ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed 2018-03-01 20:12 ` Steve French @ 2018-03-01 20:51 ` Srivatsa S. Bhat 0 siblings, 0 replies; 22+ messages in thread From: Srivatsa S. Bhat @ 2018-03-01 20:51 UTC (permalink / raw) To: Steve French Cc: Greg Kroah-Hartman, Thomas Backlund, Aurélien Aptel, LKML, Stable, Ronnie Sahlberg, Pavel Shilovskiy, CIFS 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/?h=linux-4.9.y >> >> 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 > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-03-22 19:14 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20171031095530.520746935@linuxfoundation.org>
[not found] ` <20171031095531.633196173@linuxfoundation.org>
[not found] ` <97340c9a-0ea2-0d3d-cf26-58c799d76cae@mageia.org>
[not found] ` <20171101151803.GB31285@kroah.com>
2018-01-04 2:15 ` [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed Srivatsa S. Bhat
2018-01-18 21:25 ` Srivatsa S. Bhat
2018-01-19 13:23 ` Aurélien Aptel
[not found] ` <87lggux9rp.fsf-IBi9RG/b67k@public.gmane.org>
2018-01-30 3:31 ` Srivatsa S. Bhat
2018-02-27 3:44 ` Srivatsa S. Bhat
2018-02-27 8:54 ` Greg Kroah-Hartman
2018-02-27 9:22 ` Srivatsa S. Bhat
2018-02-27 12:40 ` Greg Kroah-Hartman
2018-02-27 17:45 ` Srivatsa S. Bhat
2018-02-27 17:56 ` Steve French
2018-02-27 18:33 ` Srivatsa S. Bhat
2018-03-12 2:37 ` Steve French
2018-03-13 9:21 ` Greg Kroah-Hartman
2018-03-13 15:21 ` Steve French
2018-03-16 13:32 ` Greg Kroah-Hartman
2018-03-22 2:02 ` Steve French
2018-03-22 5:12 ` Srivatsa S. Bhat
2018-03-22 5:15 ` Srivatsa S. Bhat
2018-03-22 10:32 ` Greg Kroah-Hartman
2018-03-22 19:14 ` Pavel Shilovsky
2018-03-01 20:12 ` Steve French
2018-03-01 20:51 ` Srivatsa S. Bhat
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox