2023-06-04 3:44 GMT+09:00, 張智諺 <cc85nod@xxxxxxxxx>:
> Hello, Namjae Jeon,
Hi Chih-Yen,
Could you please check if your issue is fixed ?
Thanks!
>
> The root cause of this bug is the same as
> 3ff6bb18ebaa5458a877b47bf7dbe99100a4ff31 (ksmbd: validate smb request
> protocol id), but it occurs in compound requests.
>
> [ 8.912659] BUG: KASAN: slab-out-of-bounds in
> smb2_sess_setup+0x3ac/0x1a70
> [ 8.913081] Read of size 4 at addr ffff88800ac8bb34 by task
> kworker/0:0/7
> ...
> [ 8.914963] Call Trace:
> [ 8.915121] <TASK>
> [ 8.915261] dump_stack_lvl+0x33/0x50
> [ 8.915498] print_report+0xcc/0x620
> [ 8.916242] kasan_report+0xae/0xe0
> [ 8.916717] kasan_check_range+0x35/0x1b0
> [ 8.916965] smb2_sess_setup+0x3ac/0x1a70
> [ 8.918634] handle_ksmbd_work+0x282/0x820
> [ 8.918898] process_one_work+0x419/0x760
> [ 8.919151] worker_thread+0x2a2/0x6f0
> [ 8.919655] kthread+0x187/0x1d0
> [ 8.920165] ret_from_fork+0x1f/0x30
> [ 8.920397] </TASK>
>
> Thanks. Regards
>
From b86b7478019c8e3a1c8fe6c39fd56f7636994bbb Mon Sep 17 00:00:00 2001
From: Namjae Jeon <linkinjeon@xxxxxxxxxx>
Date: Mon, 5 Jun 2023 01:57:34 +0900
Subject: [PATCH] ksmbd: validate command payload size
->StructureSize2 indicates command payload size. ksmbd should validate
this size with rfc1002 length before accessing it.
This patch remove unneeded check and add the validation for this.
[ 8.912583] BUG: KASAN: slab-out-of-bounds in ksmbd_smb2_check_message+0x12a/0xc50
[ 8.913051] Read of size 2 at addr ffff88800ac7d92c by task kworker/0:0/7
...
[ 8.914967] Call Trace:
[ 8.915126] <TASK>
[ 8.915267] dump_stack_lvl+0x33/0x50
[ 8.915506] print_report+0xcc/0x620
[ 8.916558] kasan_report+0xae/0xe0
[ 8.917080] kasan_check_range+0x35/0x1b0
[ 8.917334] ksmbd_smb2_check_message+0x12a/0xc50
[ 8.917935] ksmbd_verify_smb_message+0xae/0xd0
[ 8.918223] handle_ksmbd_work+0x192/0x820
[ 8.918478] process_one_work+0x419/0x760
[ 8.918727] worker_thread+0x2a2/0x6f0
[ 8.919222] kthread+0x187/0x1d0
[ 8.919723] ret_from_fork+0x1f/0x30
[ 8.919954] </TASK>
Reported-by: Chih-Yen Chang <cc85nod@xxxxxxxxx>
Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
---
fs/smb/server/smb2misc.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/smb/server/smb2misc.c b/fs/smb/server/smb2misc.c
index 0ffe663b7590..57749f41b991 100644
--- a/fs/smb/server/smb2misc.c
+++ b/fs/smb/server/smb2misc.c
@@ -351,6 +351,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
int command;
__u32 clc_len; /* calculated length */
__u32 len = get_rfc1002_len(work->request_buf);
+ __u32 req_struct_size;
if (le32_to_cpu(hdr->NextCommand) > 0)
len = le32_to_cpu(hdr->NextCommand);
@@ -373,17 +374,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
}
if (smb2_req_struct_sizes[command] != pdu->StructureSize2) {
- if (command != SMB2_OPLOCK_BREAK_HE &&
- (hdr->Status == 0 || pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE)) {
- /* error packets have 9 byte structure size */
- ksmbd_debug(SMB,
- "Illegal request size %u for command %d\n",
- le16_to_cpu(pdu->StructureSize2), command);
- return 1;
- } else if (command == SMB2_OPLOCK_BREAK_HE &&
- hdr->Status == 0 &&
- le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
- le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
+ if (command == SMB2_OPLOCK_BREAK_HE &&
+ le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
+ le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
/* special case for SMB2.1 lease break message */
ksmbd_debug(SMB,
"Illegal request size %d for oplock break\n",
@@ -392,6 +385,14 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
}
}
+ req_struct_size = le16_to_cpu(pdu->StructureSize2) +
+ __SMB2_HEADER_STRUCTURE_SIZE;
+ if (command == SMB2_LOCK_HE)
+ req_struct_size -= sizeof(struct smb2_lock_element);
+
+ if (req_struct_size > len + 1)
+ return 1;
+
if (smb2_calc_size(hdr, &clc_len))
return 1;
--
2.25.1
