[PATCH] ksmbd: fix out-of-bounds read when padding the final compound response

[Ian Bridges]

Compound response handling pads the last response of a compound to an
8-byte boundary by growing the final iov and the RFC1002 length.
smb2_allocate_rsp_buf() allocates exactly response_sz. A command can
fill the response buffer to within 7 bytes of its end, for example a
QUERY_INFO whose output is bounded only by the space left. The padding
can then run past the allocation, and ksmbd_tcp_writev() reads those
bytes when it sends the response.

  BUG: KASAN: slab-out-of-bounds in _copy_from_iter+0x744/0x10e0
  Call Trace:
   _copy_from_iter+0x744/0x10e0
   tcp_sendmsg_locked+0x2290/0x3f80
   ksmbd_tcp_writev+0x10c7/0x1ab0 [ksmbd]

Allocate room for the padding when the request is a compound, as
smb2_read() and smb2_read_pipe() do for their payloads. response_sz is
unchanged, so commands still fill only up to it, and kvzalloc() zeroes
the padding.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Ian Bridges <icb@xxxxxxxxxxxx>
---

Notes:
    The bug needs a client to send a compound request whose last command
    fills the response buffer to within 7 bytes of its end, at an offset
    that is not a multiple of 8. The reproducer uses QUERY_INFO
    FILE_ALL_INFORMATION as the last command, after QUERY_DIRECTORY
    commands that fill the buffer. On a share that allows guest access
    (guest ok) it is reachable without credentials.
    
    This follows 73f860489e3b ("ksmbd: zero pipe read compound padding"),
    which fixed the same padding for smb2_read_pipe()'s payload buffer.
    
    A reproducer is available on request.
    
    This fix was developed with LLM assistance (code review and KASAN
    reproduction).
    
    Tested on ksmbd-for-next 146c3fc6540e with only this patch applied,
    under KASAN. "smb2 max trans" was lowered to 65560 so that response_sz
    ends at a KASAN redzone. Before the patch, the compound above triggers a
    KASAN slab-out-of-bounds read in _copy_from_iter() during the socket
    send. After it, the last response is still padded to 8 bytes, the
    padding is zero, QUERY_INFO returns its full output and KASAN stays
    quiet.

 fs/smb/server/smb2pdu.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index f5fe3cf8ba32..140604099bb0 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -955,7 +955,7 @@ int smb2_allocate_rsp_buf(struct ksmbd_work *work)
 	struct smb_version_values *vals = work->conn->vals;
 	size_t small_sz = MAX_CIFS_SMALL_BUFFER_SIZE;
 	size_t large_sz = small_sz + vals->max_trans_size;
-	size_t sz = small_sz;
+	size_t sz = small_sz, pad_sz = 0;
 	int cmd = le16_to_cpu(hdr->Command);
 
 	if (cmd == SMB2_IOCTL_HE || cmd == SMB2_QUERY_DIRECTORY_HE)
@@ -981,10 +981,17 @@ int smb2_allocate_rsp_buf(struct ksmbd_work *work)
 	}
 
 	/* allocate large response buf for chained commands */
-	if (le32_to_cpu(hdr->NextCommand) > 0)
+	if (le32_to_cpu(hdr->NextCommand) > 0) {
 		sz = large_sz;
+		/*
+		 * is_chained_smb2_message() pads the last response of a
+		 * compound to 8 bytes, which can take it past response_sz.
+		 * Leave room for the padding. kvzalloc() zeroes it.
+		 */
+		pad_sz = ALIGN(sz - sizeof(__be32), 8) - (sz - sizeof(__be32));
+	}
 
-	work->response_buf = kvzalloc(sz, KSMBD_DEFAULT_GFP);
+	work->response_buf = kvzalloc(sz + pad_sz, KSMBD_DEFAULT_GFP);
 	if (!work->response_buf)
 		return -ENOMEM;
 

base-commit: 146c3fc6540e24eef6296395a35bf5b924182e8b
-- 
2.47.3