From: Paulo Alcantara pc@manguebit.com
[ Upstream commit eec04ea119691e65227a97ce53c0da6b9b74b0b7 ]
Fix potential OOB in receive_encrypted_standard() if server returned a large shdr->NextCommand that would end up writing off the end of @next_buffer.
Fixes: b24df3e30cbf ("cifs: update receive_encrypted_standard to handle compounded responses") Cc: stable@vger.kernel.org Reported-by: Robert Morris rtm@csail.mit.edu Signed-off-by: Paulo Alcantara (SUSE) pc@manguebit.com Signed-off-by: Steve French stfrench@microsoft.com [Guru: receive_encrypted_standard() is present in file smb2ops.c, smb2ops.c file location is changed, modified patch accordingly.] Signed-off-by: Guruswamy Basavaiah guruswamy.basavaiah@broadcom.com --- fs/cifs/smb2ops.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index f31da2647d04..867b32b1393f 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -5153,6 +5153,7 @@ receive_encrypted_standard(struct TCP_Server_Info *server, struct smb2_sync_hdr *shdr; unsigned int pdu_length = server->pdu_size; unsigned int buf_size; + unsigned int next_cmd; struct mid_q_entry *mid_entry; int next_is_large; char *next_buffer = NULL; @@ -5181,14 +5182,15 @@ receive_encrypted_standard(struct TCP_Server_Info *server, next_is_large = server->large_buf; one_more: shdr = (struct smb2_sync_hdr *)buf; - if (shdr->NextCommand) { + next_cmd = le32_to_cpu(shdr->NextCommand); + if (next_cmd) { + if (WARN_ON_ONCE(next_cmd > pdu_length)) + return -1; if (next_is_large) next_buffer = (char *)cifs_buf_get(); else next_buffer = (char *)cifs_small_buf_get(); - memcpy(next_buffer, - buf + le32_to_cpu(shdr->NextCommand), - pdu_length - le32_to_cpu(shdr->NextCommand)); + memcpy(next_buffer, buf + next_cmd, pdu_length - next_cmd); }
mid_entry = smb2_find_mid(server, buf); @@ -5212,8 +5214,8 @@ receive_encrypted_standard(struct TCP_Server_Info *server, else ret = cifs_handle_standard(server, mid_entry);
- if (ret == 0 && shdr->NextCommand) { - pdu_length -= le32_to_cpu(shdr->NextCommand); + if (ret == 0 && next_cmd) { + pdu_length -= next_cmd; server->large_buf = next_is_large; if (next_is_large) server->bigbuf = buf = next_buffer;
From: Paulo Alcantara pc@manguebit.com
[ Upstream commit af1689a9b7701d9907dfc84d2a4b57c4bc907144 ]
Validate offsets and lengths before dereferencing create contexts in smb2_parse_contexts().
This fixes following oops when accessing invalid create contexts from server:
BUG: unable to handle page fault for address: ffff8881178d8cc3 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 4a01067 P4D 4a01067 PUD 0 Oops: 0000 [#1] PREEMPT SMP NOPTI CPU: 3 PID: 1736 Comm: mount.cifs Not tainted 6.7.0-rc4 #1 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014 RIP: 0010:smb2_parse_contexts+0xa0/0x3a0 [cifs] Code: f8 10 75 13 48 b8 93 ad 25 50 9c b4 11 e7 49 39 06 0f 84 d2 00 00 00 8b 45 00 85 c0 74 61 41 29 c5 48 01 c5 41 83 fd 0f 76 55 <0f> b7 7d 04 0f b7 45 06 4c 8d 74 3d 00 66 83 f8 04 75 bc ba 04 00 RSP: 0018:ffffc900007939e0 EFLAGS: 00010216 RAX: ffffc90000793c78 RBX: ffff8880180cc000 RCX: ffffc90000793c90 RDX: ffffc90000793cc0 RSI: ffff8880178d8cc0 RDI: ffff8880180cc000 RBP: ffff8881178d8cbf R08: ffffc90000793c22 R09: 0000000000000000 R10: ffff8880180cc000 R11: 0000000000000024 R12: 0000000000000000 R13: 0000000000000020 R14: 0000000000000000 R15: ffffc90000793c22 FS: 00007f873753cbc0(0000) GS:ffff88806bc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff8881178d8cc3 CR3: 00000000181ca000 CR4: 0000000000750ef0 PKRU: 55555554 Call Trace: <TASK> ? __die+0x23/0x70 ? page_fault_oops+0x181/0x480 ? search_module_extables+0x19/0x60 ? srso_alias_return_thunk+0x5/0xfbef5 ? exc_page_fault+0x1b6/0x1c0 ? asm_exc_page_fault+0x26/0x30 ? smb2_parse_contexts+0xa0/0x3a0 [cifs] SMB2_open+0x38d/0x5f0 [cifs] ? smb2_is_path_accessible+0x138/0x260 [cifs] smb2_is_path_accessible+0x138/0x260 [cifs] cifs_is_path_remote+0x8d/0x230 [cifs] cifs_mount+0x7e/0x350 [cifs] cifs_smb3_do_mount+0x128/0x780 [cifs] smb3_get_tree+0xd9/0x290 [cifs] vfs_get_tree+0x2c/0x100 ? capable+0x37/0x70 path_mount+0x2d7/0xb80 ? srso_alias_return_thunk+0x5/0xfbef5 ? _raw_spin_unlock_irqrestore+0x44/0x60 __x64_sys_mount+0x11a/0x150 do_syscall_64+0x47/0xf0 entry_SYSCALL_64_after_hwframe+0x6f/0x77 RIP: 0033:0x7f8737657b1e
Reported-by: Robert Morris rtm@csail.mit.edu Cc: stable@vger.kernel.org Signed-off-by: Paulo Alcantara (SUSE) pc@manguebit.com Signed-off-by: Steve French stfrench@microsoft.com [Guru: Removed changes to cached_dir.c and checking return value of smb2_parse_contexts in smb2ops.c] Signed-off-by: Guruswamy Basavaiah guruswamy.basavaiah@broadcom.com --- fs/cifs/smb2ops.c | 4 +- fs/cifs/smb2pdu.c | 93 +++++++++++++++++++++++++++------------------ fs/cifs/smb2proto.h | 12 +++--- 3 files changed, 66 insertions(+), 43 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index 867b32b1393f..0930efc7ba37 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -908,10 +908,12 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, */ kref_get(&tcon->crfid.refcount); tcon->crfid.has_lease = true; - smb2_parse_contexts(server, o_rsp, + rc = smb2_parse_contexts(server, rsp_iov, &oparms.fid->epoch, oparms.fid->lease_key, &oplock, NULL, NULL); + if (rc) + goto oshr_exit; } else goto oshr_exit;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 6714e9db0ee8..541f7d6aaf3d 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -2056,17 +2056,18 @@ parse_posix_ctxt(struct create_context *cc, struct smb2_file_all_info *info, posix->nlink, posix->mode, posix->reparse_tag); }
-void -smb2_parse_contexts(struct TCP_Server_Info *server, - struct smb2_create_rsp *rsp, - unsigned int *epoch, char *lease_key, __u8 *oplock, - struct smb2_file_all_info *buf, - struct create_posix_rsp *posix) +int smb2_parse_contexts(struct TCP_Server_Info *server, + struct kvec *rsp_iov, + unsigned int *epoch, + char *lease_key, __u8 *oplock, + struct smb2_file_all_info *buf, + struct create_posix_rsp *posix) { - char *data_offset; + struct smb2_create_rsp *rsp = rsp_iov->iov_base; struct create_context *cc; - unsigned int next; - unsigned int remaining; + size_t rem, off, len; + size_t doff, dlen; + size_t noff, nlen; char *name; static const char smb3_create_tag_posix[] = { 0x93, 0xAD, 0x25, 0x50, 0x9C, @@ -2075,45 +2076,63 @@ smb2_parse_contexts(struct TCP_Server_Info *server, };
*oplock = 0; - data_offset = (char *)rsp + le32_to_cpu(rsp->CreateContextsOffset); - remaining = le32_to_cpu(rsp->CreateContextsLength); - cc = (struct create_context *)data_offset; + + off = le32_to_cpu(rsp->CreateContextsOffset); + rem = le32_to_cpu(rsp->CreateContextsLength); + if (check_add_overflow(off, rem, &len) || len > rsp_iov->iov_len) + return -EINVAL; + cc = (struct create_context *)((u8 *)rsp + off);
/* Initialize inode number to 0 in case no valid data in qfid context */ if (buf) buf->IndexNumber = 0;
- while (remaining >= sizeof(struct create_context)) { - name = le16_to_cpu(cc->NameOffset) + (char *)cc; - if (le16_to_cpu(cc->NameLength) == 4 && - strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4) == 0) - *oplock = server->ops->parse_lease_buf(cc, epoch, - lease_key); - else if (buf && (le16_to_cpu(cc->NameLength) == 4) && - strncmp(name, SMB2_CREATE_QUERY_ON_DISK_ID, 4) == 0) - parse_query_id_ctxt(cc, buf); - else if ((le16_to_cpu(cc->NameLength) == 16)) { - if (posix && - memcmp(name, smb3_create_tag_posix, 16) == 0) + while (rem >= sizeof(*cc)) { + doff = le16_to_cpu(cc->DataOffset); + dlen = le32_to_cpu(cc->DataLength); + if (check_add_overflow(doff, dlen, &len) || len > rem) + return -EINVAL; + + noff = le16_to_cpu(cc->NameOffset); + nlen = le16_to_cpu(cc->NameLength); + if (noff + nlen >= doff) + return -EINVAL; + + name = (char *)cc + noff; + switch (nlen) { + case 4: + if (!strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4)) { + *oplock = server->ops->parse_lease_buf(cc, epoch, + lease_key); + } else if (buf && + !strncmp(name, SMB2_CREATE_QUERY_ON_DISK_ID, 4)) { + parse_query_id_ctxt(cc, buf); + } + break; + case 16: + if (posix && !memcmp(name, smb3_create_tag_posix, 16)) parse_posix_ctxt(cc, buf, posix); + break; + default: + cifs_dbg(FYI, "%s: unhandled context (nlen=%zu dlen=%zu)\n", + __func__, nlen, dlen); + if (IS_ENABLED(CONFIG_CIFS_DEBUG2)) + cifs_dump_mem("context data: ", cc, dlen); + break; } - /* else { - cifs_dbg(FYI, "Context not matched with len %d\n", - le16_to_cpu(cc->NameLength)); - cifs_dump_mem("Cctxt name: ", name, 4); - } */ - - next = le32_to_cpu(cc->Next); - if (!next) + + off = le32_to_cpu(cc->Next); + if (!off) break; - remaining -= next; - cc = (struct create_context *)((char *)cc + next); + if (check_sub_overflow(rem, off, &rem)) + return -EINVAL; + cc = (struct create_context *)((u8 *)cc + off); }
if (rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) *oplock = rsp->OplockLevel;
- return; + return 0; }
static int @@ -2983,8 +3002,8 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, }
- smb2_parse_contexts(server, rsp, &oparms->fid->epoch, - oparms->fid->lease_key, oplock, buf, posix); + rc = smb2_parse_contexts(server, &rsp_iov, &oparms->fid->epoch, + oparms->fid->lease_key, oplock, buf, posix); creat_exit: SMB2_open_free(&rqst); free_rsp_buf(resp_buftype, rsp); diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index 547945443fa7..6c5a4d44b248 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -259,11 +259,13 @@ extern int smb3_validate_negotiate(const unsigned int, struct cifs_tcon *);
extern enum securityEnum smb2_select_sectype(struct TCP_Server_Info *, enum securityEnum); -extern void smb2_parse_contexts(struct TCP_Server_Info *server, - struct smb2_create_rsp *rsp, - unsigned int *epoch, char *lease_key, - __u8 *oplock, struct smb2_file_all_info *buf, - struct create_posix_rsp *posix); +int smb2_parse_contexts(struct TCP_Server_Info *server, + struct kvec *rsp_iov, + unsigned int *epoch, + char *lease_key, __u8 *oplock, + struct smb2_file_all_info *buf, + struct create_posix_rsp *posix); + extern int smb3_encryption_required(const struct cifs_tcon *tcon); extern int smb2_validate_iov(unsigned int offset, unsigned int buffer_length, struct kvec *iov, unsigned int min_buf_size);
From: Paulo Alcantara pc@manguebit.com
[ Upstream commit 76025cc2285d9ede3d717fe4305d66f8be2d9346 ]
The data offset for the SMB3.1.1 POSIX create context will always be 8-byte aligned so having the check 'noff + nlen >= doff' in smb2_parse_contexts() is wrong as it will lead to -EINVAL because noff + nlen == doff.
Fix the sanity check to correctly handle aligned create context data.
Fixes: af1689a9b770 ("smb: client: fix potential OOBs in smb2_parse_contexts()") Signed-off-by: Paulo Alcantara pc@manguebit.com Signed-off-by: Steve French stfrench@microsoft.com [Guru:smb2_parse_contexts() is present in file smb2ops.c, smb2ops.c file location is changed, modified patch accordingly.] Signed-off-by: Guruswamy Basavaiah guruswamy.basavaiah@broadcom.com --- fs/cifs/smb2pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 541f7d6aaf3d..a358c139ba74 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -2095,7 +2095,7 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
noff = le16_to_cpu(cc->NameOffset); nlen = le16_to_cpu(cc->NameLength); - if (noff + nlen >= doff) + if (noff + nlen > doff) return -EINVAL;
name = (char *)cc + noff;
Here are the three backported patches aimed at addressing a potential crash and an actual crash.
Patch 1 Fix potential OOB access in receive_encrypted_standard() if server returned a large shdr->NextCommand in cifs.
Patch 2 fix validate offsets and lengths before dereferencing create contexts in smb2_parse_contexts().
Patch 3 fix issue in patch 2.
The original patches were authored by Paulo Alcantara pc@manguebit.com. Original Patches: 1. eec04ea11969 ("smb: client: fix OOB in receive_encrypted_standard()") 2. af1689a9b770 ("smb: client: fix potential OOBs in smb2_parse_contexts()") 3. 76025cc2285d ("smb: client: fix parsing of SMB3.1.1 POSIX create context")
Please review and consider applying these patches.
https://lore.kernel.org/all/2023121834-semisoft-snarl-49ad@gregkh/
fs/cifs/smb2ops.c | 4 +++- fs/cifs/smb2pdu.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------- fs/cifs/smb2proto.h | 12 +++++++----- 3 files changed, 66 insertions(+), 43 deletions(-)
The subject lines for patch 2/3 and patch 3/3 incorrectly mentioned "5.10.y" instead of the intended "5.15.y." These patches are intended for the 5.15.y branch, not the 5.10.y branch.
On Sun, Feb 11, 2024 at 1:43 AM Guruswamy Basavaiah guruswamy.basavaiah@broadcom.com wrote:
Here are the three backported patches aimed at addressing a potential crash and an actual crash.
Patch 1 Fix potential OOB access in receive_encrypted_standard() if server returned a large shdr->NextCommand in cifs.
Patch 2 fix validate offsets and lengths before dereferencing create contexts in smb2_parse_contexts().
Patch 3 fix issue in patch 2.
The original patches were authored by Paulo Alcantara pc@manguebit.com. Original Patches:
- eec04ea11969 ("smb: client: fix OOB in receive_encrypted_standard()")
- af1689a9b770 ("smb: client: fix potential OOBs in smb2_parse_contexts()")
- 76025cc2285d ("smb: client: fix parsing of SMB3.1.1 POSIX create context")
Please review and consider applying these patches.
https://lore.kernel.org/all/2023121834-semisoft-snarl-49ad@gregkh/
fs/cifs/smb2ops.c | 4 +++- fs/cifs/smb2pdu.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------- fs/cifs/smb2proto.h | 12 +++++++----- 3 files changed, 66 insertions(+), 43 deletions(-)
I am withdrawing the current series of patches submitted with subject "Backport Fixes to 5.15.y" to branch 5.15.y, as they had the wrong branch information in the subject line. The correct branch information will be included in the subject line and I will resend the patches shortly. I apologize for any confusion caused.
Guru
On Sun, Feb 11, 2024 at 2:25 AM Guruswamy Basavaiah guruswamy.basavaiah@broadcom.com wrote:
The subject lines for patch 2/3 and patch 3/3 incorrectly mentioned "5.10.y" instead of the intended "5.15.y." These patches are intended for the 5.15.y branch, not the 5.10.y branch.
On Sun, Feb 11, 2024 at 1:43 AM Guruswamy Basavaiah guruswamy.basavaiah@broadcom.com wrote:
Here are the three backported patches aimed at addressing a potential crash and an actual crash.
Patch 1 Fix potential OOB access in receive_encrypted_standard() if server returned a large shdr->NextCommand in cifs.
Patch 2 fix validate offsets and lengths before dereferencing create contexts in smb2_parse_contexts().
Patch 3 fix issue in patch 2.
The original patches were authored by Paulo Alcantara pc@manguebit.com. Original Patches:
- eec04ea11969 ("smb: client: fix OOB in receive_encrypted_standard()")
- af1689a9b770 ("smb: client: fix potential OOBs in smb2_parse_contexts()")
- 76025cc2285d ("smb: client: fix parsing of SMB3.1.1 POSIX create context")
Please review and consider applying these patches.
https://lore.kernel.org/all/2023121834-semisoft-snarl-49ad@gregkh/
fs/cifs/smb2ops.c | 4 +++- fs/cifs/smb2pdu.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------- fs/cifs/smb2proto.h | 12 +++++++----- 3 files changed, 66 insertions(+), 43 deletions(-)
linux-stable-mirror@lists.linaro.org
-
Guruswamy Basavaiah