[PATCH] ksmbd: fix SMB2 CREATE response buffer overflow

Jérémy Jean posted 1 patch 5 days, 4 hours ago
There is a newer version of this series
fs/smb/server/smb2pdu.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
[PATCH] ksmbd: fix SMB2 CREATE response buffer overflow
Posted by Jérémy Jean 5 days, 4 hours ago
smb2_allocate_rsp_buf() uses the MAX_CIFS_SMALL_BUFFER_SIZE
(448-byte) buffer for a single SMB2_CREATE response. That buffer also
holds a 4-byte length field, which only leaves 444 bytes for the SMB2
body.

The AAPL response made this buffer too small. After 8f1b796ff113, a
request with AAPL response contexts may need at least 456 bytes. In
the 456-byte case, create_aapl_rsp_buf() is appended last, clears 128
bytes, and writes 12 bytes past the allocation.

KASAN reports:

    BUG: KASAN: slab-out-of-bounds in create_aapl_rsp_buf+0x31/0x6e0
    Write of size 128 at addr ffff88800370954c by task kworker/0:0/9
    ...
    create_aapl_rsp_buf+0x31/0x6e0
    smb2_open+0x58f9/0xef10
    ...
    smb2_allocate_rsp_buf+0x19d/0x370
    ...
    The buggy address is located 332 bytes inside of
     allocated 448-byte region [ffff888003709400, ffff8880037095c0)

Reserve enough space for any fixed CREATE response KSMBD can build. This
keeps the small buffer for other commands and avoids using the max
transaction buffer for every CREATE.

Fixes: 8f1b796ff113 ("ksmbd: add AAPL kAAPL_SERVER_QUERY create context support")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 fs/smb/server/smb2pdu.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index b7ce67094626..24a3fd463be0 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -860,14 +860,37 @@ static void smb2_update_lock_sequence(struct ksmbd_work *work,
 int smb2_allocate_rsp_buf(struct ksmbd_work *work)
 {
 	struct smb2_hdr *hdr = smb_get_msg(work->request_buf);
+	struct smb_version_values *vals = work->conn->vals;
 	size_t small_sz = MAX_CIFS_SMALL_BUFFER_SIZE;
-	size_t large_sz = small_sz + work->conn->vals->max_trans_size;
+	size_t large_sz = small_sz + vals->max_trans_size;
 	size_t sz = small_sz;
 	int cmd = le16_to_cpu(hdr->Command);
 
 	if (cmd == SMB2_IOCTL_HE || cmd == SMB2_QUERY_DIRECTORY_HE)
 		sz = large_sz;
 
+	if (cmd == SMB2_CREATE_HE) {
+		size_t create_sz;
+
+		/*
+		 * A CREATE response may contain any combination of fixed
+		 * response contexts. Account for the 4-byte length field
+		 * because work->response_sz includes it while smb_get_msg()
+		 * skips over it.
+		 */
+		create_sz = sizeof(__be32) +
+			    offsetof(struct smb2_create_rsp, Buffer);
+		create_sz += vals->create_lease_size;
+		create_sz += max_t(size_t,
+				   vals->create_durable_size,
+				   vals->create_durable_v2_size);
+		create_sz += vals->create_mxac_size;
+		create_sz += vals->create_disk_id_size;
+		create_sz += vals->create_posix_size;
+		create_sz += vals->create_aapl_size;
+		sz = max_t(size_t, sz, create_sz);
+	}
+
 	if (cmd == SMB2_QUERY_INFO_HE) {
 		struct smb2_query_info_req *req;
 
-- 
2.47.3

Re: [PATCH] ksmbd: fix SMB2 CREATE response buffer overflow
Posted by Namjae Jeon 4 days, 19 hours ago
> +       if (cmd == SMB2_CREATE_HE) {
> +               size_t create_sz;
> +
> +               /*
> +                * A CREATE response may contain any combination of fixed
> +                * response contexts. Account for the 4-byte length field
> +                * because work->response_sz includes it while smb_get_msg()
> +                * skips over it.
> +                */
> +               create_sz = sizeof(__be32) +
> +                           offsetof(struct smb2_create_rsp, Buffer);
> +               create_sz += vals->create_lease_size;
> +               create_sz += max_t(size_t,
> +                                  vals->create_durable_size,
> +                                  vals->create_durable_v2_size);
> +               create_sz += vals->create_mxac_size;
> +               create_sz += vals->create_disk_id_size;
> +               create_sz += vals->create_posix_size;
> +               create_sz += vals->create_aapl_size;
> +               sz = max_t(size_t, sz, create_sz);
Since these sizes are fixed for each SMB dialect, we should avoid
recalculating them for every request. It would be cleaner to define
the required CREATE response size in advance and use that predefined
value during buffer allocation.
Thanks.