* [PATCH 5.10.y 1/3] smb: client: fix OOB in receive_encrypted_standard()
@ 2024-02-10 20:14 Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 2/3] smb: client: fix potential OOBs in smb2_parse_contexts() Guruswamy Basavaiah
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Guruswamy Basavaiah @ 2024-02-10 20:14 UTC (permalink / raw
To: stable, gregkh
Cc: ajay.kaher, tapas.kundu, Guruswamy Basavaiah, Robert Morris,
Paulo Alcantara, Steve French
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 26edaeb4245d..5ba7056c78c7 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4892,6 +4892,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;
@@ -4920,14 +4921,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);
@@ -4951,8 +4953,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;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 5.10.y 2/3] smb: client: fix potential OOBs in smb2_parse_contexts()
2024-02-10 20:14 [PATCH 5.10.y 1/3] smb: client: fix OOB in receive_encrypted_standard() Guruswamy Basavaiah
@ 2024-02-10 20:14 ` Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 3/3] smb: client: fix parsing of SMB3.1.1 POSIX create context Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 0/3] Backport Fixes to 5.10.y Guruswamy Basavaiah
2 siblings, 0 replies; 5+ messages in thread
From: Guruswamy Basavaiah @ 2024-02-10 20:14 UTC (permalink / raw
To: stable, gregkh
Cc: ajay.kaher, tapas.kundu, Guruswamy Basavaiah, Robert Morris,
Paulo Alcantara, Steve French
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 5ba7056c78c7..05b06e5e6da1 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -818,10 +818,12 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon,
if (o_rsp->OplockLevel == SMB2_OPLOCK_LEVEL_LEASE) {
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 76679dc4e632..71df2357c64f 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1992,17 +1992,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,
@@ -2011,45 +2012,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
@@ -2916,8 +2935,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 ed2b4fb012a4..3184a5efcdba 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -270,11 +270,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);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 5.10.y 3/3] smb: client: fix parsing of SMB3.1.1 POSIX create context
2024-02-10 20:14 [PATCH 5.10.y 1/3] smb: client: fix OOB in receive_encrypted_standard() Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 2/3] smb: client: fix potential OOBs in smb2_parse_contexts() Guruswamy Basavaiah
@ 2024-02-10 20:14 ` Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 0/3] Backport Fixes to 5.10.y Guruswamy Basavaiah
2 siblings, 0 replies; 5+ messages in thread
From: Guruswamy Basavaiah @ 2024-02-10 20:14 UTC (permalink / raw
To: stable, gregkh
Cc: ajay.kaher, tapas.kundu, Guruswamy Basavaiah, Paulo Alcantara,
Steve French
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 71df2357c64f..5b49c36bd00d 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -2031,7 +2031,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;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 5.10.y 0/3] Backport Fixes to 5.10.y
2024-02-10 20:14 [PATCH 5.10.y 1/3] smb: client: fix OOB in receive_encrypted_standard() Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 2/3] smb: client: fix potential OOBs in smb2_parse_contexts() Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 3/3] smb: client: fix parsing of SMB3.1.1 POSIX create context Guruswamy Basavaiah
@ 2024-02-10 20:14 ` Guruswamy Basavaiah
2024-02-23 13:07 ` Greg KH
2 siblings, 1 reply; 5+ messages in thread
From: Guruswamy Basavaiah @ 2024-02-10 20:14 UTC (permalink / raw
To: stable, gregkh; +Cc: ajay.kaher, tapas.kundu
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(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5.10.y 0/3] Backport Fixes to 5.10.y
2024-02-10 20:14 ` [PATCH 5.10.y 0/3] Backport Fixes to 5.10.y Guruswamy Basavaiah
@ 2024-02-23 13:07 ` Greg KH
0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2024-02-23 13:07 UTC (permalink / raw
To: Guruswamy Basavaiah; +Cc: stable, ajay.kaher, tapas.kundu
On Sun, Feb 11, 2024 at 01:44:45AM +0530, Guruswamy Basavaiah 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:
> 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.
All now qeued up, thanks!
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-23 13:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-10 20:14 [PATCH 5.10.y 1/3] smb: client: fix OOB in receive_encrypted_standard() Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 2/3] smb: client: fix potential OOBs in smb2_parse_contexts() Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 3/3] smb: client: fix parsing of SMB3.1.1 POSIX create context Guruswamy Basavaiah
2024-02-10 20:14 ` [PATCH 5.10.y 0/3] Backport Fixes to 5.10.y Guruswamy Basavaiah
2024-02-23 13:07 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.