2023-05-21 12:32 GMT+09:00, 張智諺 <cc85nod@xxxxxxxxx>:
> Ok, I will update my patch and resend it to you.
>
> One more thing I want to check it with you, if ksmbd needs to check the
> protocol id, does it need to
> receive first 4 bytes of smb2_hdr first to check the ProtocolID? Or how can
> I properly check that ProtocolID is SMB2?
Yes, We need to check it after reading. we can check pdu size is
smaller than smb1 header before reading and check smb2 header after
reading. Let me know if there is more good way.
diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index 4882a812ea86..b61d1ac94146 100644
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -294,6 +294,9 @@ bool ksmbd_conn_alive(struct ksmbd_conn *conn)
return true;
}
+#define SMB1_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb_hdr))
+#define SMB2_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr) + 4)
+
/**
* ksmbd_conn_handler_loop() - session thread to listen on new smb requests
* @p: connection instance
@@ -353,6 +356,10 @@ int ksmbd_conn_handler_loop(void *p)
/* 4 for rfc1002 length field */
/* 1 for implied bcc[0] */
size = pdu_size + 4 + 1;
+
+ if (pdu_size < SMB1_MIN_SUPPORTED_HEADER_SIZE)
+ break;
+
conn->request_buf = kvmalloc(size, GFP_KERNEL);
if (!conn->request_buf)
break;
@@ -377,6 +384,12 @@ int ksmbd_conn_handler_loop(void *p)
continue;
}
+ if (((struct smb2_hdr
*)smb2_get_msg(conn->request_buf))->ProtocolId ==
+ SMB2_PROTO_NUMBER) {
+ if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
+ break;
+ }
+
if (!default_conn_ops.process_fn) {
pr_err("No connection request callback\n");
break;
>
> Thanks.
>
> Namjae Jeon <linkinjeon@xxxxxxxxxx> 於 2023年5月21日 週日 上午9:22寫道：
>
>> 2023-05-21 2:25 GMT+09:00, 張智諺 <cc85nod@xxxxxxxxx>:
>> > From ec0d45674df77bd3344717de3eae68de70210829 Mon Sep 17 00:00:00 2001
>> > From: Pumpkin <cc85nod@xxxxxxxxx>
>> > Date: Sun, 21 May 2023 01:04:41 +0800
>> > Subject: [PATCH] ksmbd: check the validation of pdu_size in
>> > ksmbd_conn_handler_loop
>> >
>> > The length field of netbios header must be greater than the SMB header
>> > size, otherwise the packet is an invalid SMB packet.
>> >
>> > If `pdu_size` is 0, ksmbd allocates a 4 bytes chunk to
>> `conn->request_buf`.
>> > In the function `get_smb2_cmd_val` ksmbd will read cmd from
>> > `rcv_hdr->Command`,
>> > which is `conn->request_buf + 12`, causing the KASAN detector to print
>> the
>> > following error message:
>> >
>> > [ 7.205018] BUG: KASAN: slab-out-of-bounds in
>> get_smb2_cmd_val+0x45/0x60
>> > [ 7.205423] Read of size 2 at addr ffff8880062d8b50 by task
>> > ksmbd:42632/248
>> > ...
>> > [ 7.207125] <TASK>
>> > [ 7.209191] get_smb2_cmd_val+0x45/0x60
>> > [ 7.209426] ksmbd_conn_enqueue_request+0x3a/0x100
>> > [ 7.209712] ksmbd_server_process_request+0x72/0x160
>> > [ 7.210295] ksmbd_conn_handler_loop+0x30c/0x550
>> > [ 7.212280] kthread+0x160/0x190
>> > [ 7.212762] ret_from_fork+0x1f/0x30
>> > [ 7.212981] </TASK>
>> >
>> > Signed-off-by: Pumpkin <cc85nod@xxxxxxxxx>
>> Real name? please change your gitconfig.
>> > ---
>> > fs/ksmbd/connection.c | 5 +++++
>> > 1 file changed, 5 insertions(+)
>> >
>> > diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
>> > index 4ed379f9b..2a7748694 100644
>> > --- a/fs/ksmbd/connection.c
>> > +++ b/fs/ksmbd/connection.c
>> > @@ -343,6 +343,11 @@ int ksmbd_conn_handler_loop(void *p)
>> > READ_ONCE(conn->status));
>> > break;
>> > }
>> > +
>> > + if (pdu_size < sizeof(struct smb2_hdr)) {
>> > + pr_err("PDU length(%u) is less than SMB header size\n", size);
>> > + break;
>> > + }
>> OK. It will block smb1 negotiate request, We need to allow only smb1
>> negotiate in smb1 commands for auto negotiation. It will cause windows
>> client connection failure. So We need to check if protocol id is smb2
>> for this check. and +4 is needed for rfc1002 length If you want
>> pdu_size to be greater than or equal to the smb2 header size. i.e.
>> declare the macro like the following.
>>
>> #define KSMBD_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr) + 4)
>>
>> Thanks for your work!
>> >
>> > /*
>> > * Check maximum pdu size(0x00FFFFFF).
>> > --
>> > 2.39.2 (Apple Git-143)
>> >
>>
>
