Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 4 Sep 2024 19:58:22 GMT
From:      Ed Maste <emaste@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 29fbeb5c59c7 - releng/13.4 - ctl: fix Use-After-Free in ctl_write_buffer
Message-ID:  <202409041958.484JwMHt029932@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch releng/13.4 has been updated by emaste:

URL: https://cgit.FreeBSD.org/src/commit/?id=29fbeb5c59c7cadd6bb3b9a1dda3441689ad5eb6

commit 29fbeb5c59c7cadd6bb3b9a1dda3441689ad5eb6
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2024-09-04 14:38:11 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-09-04 19:13:45 +0000

    ctl: fix Use-After-Free in ctl_write_buffer
    
    The virtio_scsi device allows a guest VM to directly send SCSI commands
    to the kernel driver exposed on /dev/cam/ctl. This setup makes the
    vulnerability directly accessible from VMs through the pci_virtio_scsi
    bhyve device.
    
    The function ctl_write_buffer sets the CTL_FLAG_ALLOCATED flag, causing
    the kern_data_ptr to be freed when the command finishes processing.
    However, the buffer is still stored in lun->write_buffer, leading to a
    Use-After-Free vulnerability.
    
    Since the buffer needs to persist indefinitely, so it can be accessed by
    READ BUFFER, do not set CTL_FLAG_ALLOCATED.
    
    Reported by:    Synacktiv
    Reviewed by:    Pierre Pronchery <pierre@freebsdfoundation.org>
    Reviewed by:    jhb
    Security:       FreeBSD-SA-24:11.ctl
    Security:       CVE-2024-45063
    Security:       HYP-03
    Sponsored by:   Axcient
    Sponsored by:   The Alpha-Omega Project
    Sponsored by:   The FreeBSD Foundation
    Differential Revision: https://reviews.freebsd.org/D46424
    
    (cherry picked from commit 670b582db6cb827a8760df942ed8af0020a0b4d0)
    (cherry picked from commit 29937d7a1a0a3061c6ae12b5b35cc32b03829501)
    (cherry picked from commit 2553c91f731b12558046454cf30eb83f56fb204b)
    
    Approved by:    so
    Approved by:    re (cperciva)
---
 sys/cam/ctl/ctl.c         | 19 +++++++++++--------
 sys/cam/ctl/ctl_private.h |  8 ++++++++
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/sys/cam/ctl/ctl.c b/sys/cam/ctl/ctl.c
index 7a04a668e823..5635246845b9 100644
--- a/sys/cam/ctl/ctl.c
+++ b/sys/cam/ctl/ctl.c
@@ -5672,21 +5672,24 @@ ctl_write_buffer(struct ctl_scsiio *ctsio)
 		return (CTL_RETVAL_COMPLETE);
 	}
 
+	if (lun->write_buffer == NULL) {
+		lun->write_buffer = malloc(CTL_WRITE_BUFFER_SIZE,
+		    M_CTL, M_WAITOK);
+	}
+
 	/*
-	 * If we've got a kernel request that hasn't been malloced yet,
-	 * malloc it and tell the caller the data buffer is here.
+	 * If this kernel request hasn't started yet, initialize the data
+	 * buffer to the correct region of the LUN's write buffer.  Note that
+	 * this doesn't set CTL_FLAG_ALLOCATED since this points into a
+	 * persistent buffer belonging to the LUN rather than a buffer
+	 * dedicated to this request.
 	 */
-	if ((ctsio->io_hdr.flags & CTL_FLAG_ALLOCATED) == 0) {
-		if (lun->write_buffer == NULL) {
-			lun->write_buffer = malloc(CTL_WRITE_BUFFER_SIZE,
-			    M_CTL, M_WAITOK);
-		}
+	if (ctsio->kern_data_ptr == NULL) {
 		ctsio->kern_data_ptr = lun->write_buffer + buffer_offset;
 		ctsio->kern_data_len = len;
 		ctsio->kern_total_len = len;
 		ctsio->kern_rel_offset = 0;
 		ctsio->kern_sg_entries = 0;
-		ctsio->io_hdr.flags |= CTL_FLAG_ALLOCATED;
 		ctsio->be_move_done = ctl_config_move_done;
 		ctl_datamove((union ctl_io *)ctsio);
 
diff --git a/sys/cam/ctl/ctl_private.h b/sys/cam/ctl/ctl_private.h
index 9a87345015fa..85f4f6137810 100644
--- a/sys/cam/ctl/ctl_private.h
+++ b/sys/cam/ctl/ctl_private.h
@@ -410,6 +410,14 @@ struct ctl_lun {
 	uint8_t				pr_res_type;
 	int				prevent_count;
 	uint32_t			*prevent;
+
+	/*
+	 * The READ_BUFFER and WRITE_BUFFER commands permit access to a logical
+	 * data buffer associated with a LUN.  Accesses to the data buffer do
+	 * not affect data stored on the storage medium.  To support this,
+	 * allocate a buffer on first use that persists until the LUN is
+	 * destroyed.
+	 */
 	uint8_t				*write_buffer;
 	struct ctl_devid		*lun_devid;
 	TAILQ_HEAD(tpc_lists, tpc_list) tpc_lists;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202409041958.484JwMHt029932>