Date: Wed, 04 Mar 2026 17:30:21 +0000 From: Ed Maste <emaste@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Cc: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org> Subject: git: 4dfeadc32e46 - main - bhyve/virtio-scsi: Check LUN address validity Message-ID: <69a86c2d.409ab.42ac10c5@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=4dfeadc32e464557c2aa450212ac419bc567d1e6 commit 4dfeadc32e464557c2aa450212ac419bc567d1e6 Author: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org> AuthorDate: 2025-10-28 08:51:26 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2026-03-04 17:30:02 +0000 bhyve/virtio-scsi: Check LUN address validity Instead of blindly trusting the guest OS driver that it sends us well- formed LUN addresses, check the LUN address for validity and fail the request if it is invalid. While here, constify the members of the virtio requests which aren't device-writable anyway. Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D53470 --- usr.sbin/bhyve/pci_virtio_scsi.c | 123 ++++++++++++++++++++++++++++++++++----- 1 file changed, 108 insertions(+), 15 deletions(-) diff --git a/usr.sbin/bhyve/pci_virtio_scsi.c b/usr.sbin/bhyve/pci_virtio_scsi.c index 758e2643f6a0..dafff50fa531 100644 --- a/usr.sbin/bhyve/pci_virtio_scsi.c +++ b/usr.sbin/bhyve/pci_virtio_scsi.c @@ -200,10 +200,10 @@ struct pci_vtscsi_softc { #define VIRTIO_SCSI_S_FUNCTION_REJECTED 11 struct pci_vtscsi_ctrl_tmf { - uint32_t type; - uint32_t subtype; - uint8_t lun[8]; - uint64_t id; + const uint32_t type; + const uint32_t subtype; + const uint8_t lun[8]; + const uint64_t id; uint8_t response; } __attribute__((packed)); @@ -216,9 +216,9 @@ struct pci_vtscsi_ctrl_tmf { #define VIRTIO_SCSI_EVT_ASYNC_DEVICE_BUSY 64 struct pci_vtscsi_ctrl_an { - uint32_t type; - uint8_t lun[8]; - uint32_t event_requested; + const uint32_t type; + const uint8_t lun[8]; + const uint32_t event_requested; uint32_t event_actual; uint8_t response; } __attribute__((packed)); @@ -249,12 +249,12 @@ struct pci_vtscsi_event { } __attribute__((packed)); struct pci_vtscsi_req_cmd_rd { - uint8_t lun[8]; - uint64_t id; - uint8_t task_attr; - uint8_t prio; - uint8_t crn; - uint8_t cdb[]; + const uint8_t lun[8]; + const uint64_t id; + const uint8_t task_attr; + const uint8_t prio; + const uint8_t crn; + const uint8_t cdb[]; } __attribute__((packed)); struct pci_vtscsi_req_cmd_wr { @@ -271,7 +271,10 @@ static void pci_vtscsi_reset(void *); static void pci_vtscsi_neg_features(void *, uint64_t); static int pci_vtscsi_cfgread(void *, int, int, uint32_t *); static int pci_vtscsi_cfgwrite(void *, int, int, uint32_t); -static inline int pci_vtscsi_get_lun(uint8_t *); + +static inline bool pci_vtscsi_check_lun(const uint8_t *); +static inline int pci_vtscsi_get_lun(const uint8_t *); + static void pci_vtscsi_control_handle(struct pci_vtscsi_softc *, void *, size_t); static void pci_vtscsi_tmf_handle(struct pci_vtscsi_softc *, struct pci_vtscsi_ctrl_tmf *); @@ -398,9 +401,76 @@ pci_vtscsi_cfgwrite(void *vsc __unused, int offset __unused, int size __unused, return (0); } +/* + * LUN address parsing + * + * The LUN address consists of 8 bytes. While the spec describes this as 0x01, + * followed by the target byte, followed by a "single-level LUN structure", + * this is actually the same as a hierarchical LUN address as defined by SAM-5, + * consisting of four levels of addressing, where in each level the two MSB of + * byte 0 select the address mode used in the remaining bits and bytes. + * + * + * Only the first two levels are acutally used by virtio-scsi: + * + * Level 1: 0x01, 0xTT: Peripheral Device Addressing: Bus 1, Target 0-255 + * Level 2: 0xLL, 0xLL: Peripheral Device Addressing: Bus MBZ, LUN 0-255 + * or: Flat Space Addressing: LUN (0-16383) + * Level 3 and 4: not used, MBZ + * + * Currently, we only support Target 0. + * + * Alternatively, the first level may contain an extended LUN address to select + * the REPORT_LUNS well-known logical unit: + * + * Level 1: 0xC1, 0x01: Extended LUN Adressing, Well-Known LUN 1 (REPORT_LUNS) + * Level 2, 3, and 4: not used, MBZ + * + * The virtio spec says that we SHOULD implement the REPORT_LUNS well-known + * logical unit but we currently don't. + * + * According to the virtio spec, these are the only LUNS address formats to be + * used with virtio-scsi. + */ + +/* + * Check that the given LUN address conforms to the virtio spec, does not + * address an unknown target, and especially does not address the REPORT_LUNS + * well-known logical unit. + */ +static inline bool +pci_vtscsi_check_lun(const uint8_t *lun) +{ + if (lun[0] == 0xC1) + return (false); + + if (lun[0] != 0x01) + return (false); + + if (lun[1] != 0x00) + return (false); + + if (lun[2] != 0x00 && (lun[2] & 0xc0) != 0x40) + return (false); + + if (lun[4] != 0 || lun[5] != 0 || lun[6] != 0 || lun[7] != 0) + return (false); + + return (true); +} + +/* + * Get the LUN id from a LUN address. + * + * Every code path using this function must have called pci_vtscsi_check_lun() + * before to make sure the LUN address is valid. + */ static inline int -pci_vtscsi_get_lun(uint8_t *lun) +pci_vtscsi_get_lun(const uint8_t *lun) { + assert(lun[0] == 0x01); + assert(lun[1] == 0x00); + assert(lun[2] == 0x00 || (lun[2] & 0xc0) == 0x40); return (((lun[2] << 8) | lun[3]) & 0x3fff); } @@ -444,6 +514,16 @@ pci_vtscsi_tmf_handle(struct pci_vtscsi_softc *sc, union ctl_io *io; int err; + if (pci_vtscsi_check_lun(tmf->lun) == false) { + DPRINTF("TMF request to invalid LUN %.2hhx%.2hhx-%.2hhx%.2hhx-" + "%.2hhx%.2hhx-%.2hhx%.2hhx", tmf->lun[0], tmf->lun[1], + tmf->lun[2], tmf->lun[3], tmf->lun[4], tmf->lun[5], + tmf->lun[6], tmf->lun[7]); + + tmf->response = VIRTIO_SCSI_S_BAD_TARGET; + return; + } + io = ctl_scsi_alloc_io(sc->vss_iid); if (io == NULL) { WPRINTF("failed to allocate ctl_io: err=%d (%s)", @@ -670,6 +750,19 @@ pci_vtscsi_queue_request(struct pci_vtscsi_softc *sc, struct vqueue_info *vq) assert(iov_to_buf(req->vsr_iov_in, req->vsr_niov_in, (void **)&req->vsr_cmd_rd) == VTSCSI_IN_HEADER_LEN(q->vsq_sc)); + /* Make sure this request addresses a valid LUN. */ + if (pci_vtscsi_check_lun(req->vsr_cmd_rd->lun) == false) { + DPRINTF("I/O request to invalid LUN " + "%.2hhx%.2hhx-%.2hhx%.2hhx-%.2hhx%.2hhx-%.2hhx%.2hhx", + req->vsr_cmd_rd->lun[0], req->vsr_cmd_rd->lun[1], + req->vsr_cmd_rd->lun[2], req->vsr_cmd_rd->lun[3], + req->vsr_cmd_rd->lun[4], req->vsr_cmd_rd->lun[5], + req->vsr_cmd_rd->lun[6], req->vsr_cmd_rd->lun[7]); + req->vsr_cmd_wr->response = VIRTIO_SCSI_S_BAD_TARGET; + pci_vtscsi_return_request(q, req, 1); + return; + } + pthread_mutex_lock(&q->vsq_rmtx); pci_vtscsi_put_request(&q->vsq_requests, req); pthread_cond_signal(&q->vsq_cv);home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?69a86c2d.409ab.42ac10c5>