Hi Aurélien, On 1/19/18 5:23 AM, Aurélien Aptel wrote: > Hi, > > "Srivatsa S. Bhat" <srivatsa@xxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html