ksmbd: zero the smb2_read alignment tail to avoid an infoleak

Commit 6b9a2e09d4cc ("ksmbd: avoid zeroing the read buffer in smb2_read()")
switched the SMB2 READ payload buffer from kvzalloc() to kvmalloc(), on the
premise that only the nbytes actually read are ever transmitted, so the
ALIGN(length, 8) tail need not be initialized.

That premise does not hold for a compound response. ksmbd_vfs_read() fills
only nbytes, leaving [nbytes, ALIGN(length, 8)) uninitialized. The aux
payload is pinned as the last response iov with iov_len == nbytes, but when
the READ is a member of a compound, init_chained_smb2_rsp() 8-byte-aligns
the previous member by extending that same iov:

	new_len = ALIGN(len, 8);
	work->iov[work->iov_idx].iov_len += (new_len - len);
	inc_rfc1001_len(work->response_buf, new_len - len);

so up to 7 uninitialized bytes of the kvmalloc()'d slab tail are sent
to the client. When the read length is small the buffer is served from
a general kmalloc slab, so those bytes can be stale kernel-heap
contents, including pointer values -- an information leak usable to
defeat KASLR.

An authenticated client triggers it with a compound request containing a
READ whose returned nbytes is not 8-aligned (for example [READ, CLOSE] with
a 1-byte read).

Zero only the alignment tail after the read, preserving the bulk
no-zeroing optimization of 6b9a2e09d4cc.

Fixes: 6b9a2e09d4cc ("ksmbd: avoid zeroing the read buffer in smb2_read()")
Cc: stable@vger.kernel.org
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 320b46db9377954c54d9006b37c99d1b154400a3..c407425cbba016614b6c58866e1a225bc16cb637 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -7446,6 +7446,15 @@ int smb2_read(struct ksmbd_work *work)
                goto out;
        }
 
+       /*
+        * ksmbd_vfs_read() fills only nbytes; the [nbytes, ALIGN(nbytes, 8))
+        * tail of the un-zeroed buffer is transmitted as compound-response
+        * alignment padding, leaking uninitialized kernel memory to the
+        * client.  Zero just that tail.
+        */
+       if (nbytes & 7)
+               memset(aux_payload_buf + nbytes, 0, ALIGN(nbytes, 8) - nbytes);
+
        if ((nbytes == 0 && length != 0) || nbytes < mincount) {
                kvfree(aux_payload_buf);
                rsp->hdr.Status = STATUS_END_OF_FILE;