Skip to content

Commit 2b9b8f3

Browse files
namjaejeonSteve French
authored andcommitted
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> Cc: stable@vger.kernel.org Reported-by: Chih-Yen Chang <cc85nod@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 858fd16 commit 2b9b8f3

File tree

1 file changed

+12
-11
lines changed

1 file changed

+12
-11
lines changed

fs/smb/server/smb2misc.c

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
351351
int command;
352352
__u32 clc_len; /* calculated length */
353353
__u32 len = get_rfc1002_len(work->request_buf);
354+
__u32 req_struct_size;
354355

355356
if (le32_to_cpu(hdr->NextCommand) > 0)
356357
len = le32_to_cpu(hdr->NextCommand);
@@ -373,17 +374,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
373374
}
374375

375376
if (smb2_req_struct_sizes[command] != pdu->StructureSize2) {
376-
if (command != SMB2_OPLOCK_BREAK_HE &&
377-
(hdr->Status == 0 || pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE)) {
378-
/* error packets have 9 byte structure size */
379-
ksmbd_debug(SMB,
380-
"Illegal request size %u for command %d\n",
381-
le16_to_cpu(pdu->StructureSize2), command);
382-
return 1;
383-
} else if (command == SMB2_OPLOCK_BREAK_HE &&
384-
hdr->Status == 0 &&
385-
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
386-
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
377+
if (command == SMB2_OPLOCK_BREAK_HE &&
378+
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
379+
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
387380
/* special case for SMB2.1 lease break message */
388381
ksmbd_debug(SMB,
389382
"Illegal request size %d for oplock break\n",
@@ -392,6 +385,14 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
392385
}
393386
}
394387

388+
req_struct_size = le16_to_cpu(pdu->StructureSize2) +
389+
__SMB2_HEADER_STRUCTURE_SIZE;
390+
if (command == SMB2_LOCK_HE)
391+
req_struct_size -= sizeof(struct smb2_lock_element);
392+
393+
if (req_struct_size > len + 1)
394+
return 1;
395+
395396
if (smb2_calc_size(hdr, &clc_len))
396397
return 1;
397398

0 commit comments

Comments
 (0)