git: e7439f6aeb23 - main - bhyve xhci: Cache the value of MaxPStreams when initializing an endpoint.

The branch main has been updated by jhb:

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

commit e7439f6aeb235ba3a7e79818c56a63d066c80854
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2022-08-17 17:00:36 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2022-08-17 17:00:36 +0000

    bhyve xhci: Cache the value of MaxPStreams when initializing an endpoint.
    
    This avoids type confusion where a malicious guest could rewrite the
    MaxPStreams field in an endpoint context after the endpoint was
    initialized causing the device model to interpret a guest provided
    address (stored in ep_ringaddr of the "software" endpoint state) as a
    bhyve host process address (ep_sctx_trbs).  It also prevents a malicious
    guest from triggering overflows of ep_sctx_trbs[] by increasing the
    number of streams after the endpoint has been initialized.
    
    Rather than re-reading the MaxPStreams value out of the endpoint context
    in guest memory on subsequent operations, cache the value in the software
    endpoint state.  Possibly the device model should raise errors if the
    value of MaxPStreams changes while an endpoint is running.  This approach
    simply ignores any such changes by the guest.
    
    PR:             264294, 264347
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    markj
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D36181
---
 usr.sbin/bhyve/pci_xhci.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/usr.sbin/bhyve/pci_xhci.c b/usr.sbin/bhyve/pci_xhci.c
index 02a9b8f0b356..bea5095784a3 100644
--- a/usr.sbin/bhyve/pci_xhci.c
+++ b/usr.sbin/bhyve/pci_xhci.c
@@ -170,6 +170,13 @@ struct pci_xhci_dev_ep {
 #define	ep_tr		_ep_trbsctx._epu_tr
 #define	ep_sctx		_ep_trbsctx._epu_sctx
 
+	/*
+	 * Caches the value of MaxPStreams from the endpoint context
+	 * when an endpoint is initialized and is used to validate the
+	 * use of ep_ringaddr vs ep_sctx_trbs[] as well as the length
+	 * of ep_sctx_trbs[].
+	 */
+	uint32_t ep_MaxPStreams;
 	union {
 		struct pci_xhci_trb_ring _epu_trb;
 		struct pci_xhci_trb_ring *_epu_sctx_trbs;
@@ -673,6 +680,7 @@ pci_xhci_init_ep(struct pci_xhci_dev_emu *dev, int epid)
 		devep->ep_tr = XHCI_GADDR(dev->xsc, devep->ep_ringaddr);
 		DPRINTF(("init_ep tr DCS %x", devep->ep_ccs));
 	}
+	devep->ep_MaxPStreams = pstreams;
 
 	if (devep->ep_xfer == NULL) {
 		devep->ep_xfer = malloc(sizeof(struct usb_data_xfer));
@@ -694,9 +702,8 @@ pci_xhci_disable_ep(struct pci_xhci_dev_emu *dev, int epid)
 	ep_ctx->dwEpCtx0 = (ep_ctx->dwEpCtx0 & ~0x7) | XHCI_ST_EPCTX_DISABLED;
 
 	devep = &dev->eps[epid];
-	if (XHCI_EPCTX_0_MAXP_STREAMS_GET(ep_ctx->dwEpCtx0) > 0 &&
-	    devep->ep_sctx_trbs != NULL)
-			free(devep->ep_sctx_trbs);
+	if (devep->ep_MaxPStreams > 0)
+		free(devep->ep_sctx_trbs);
 
 	if (devep->ep_xfer != NULL) {
 		free(devep->ep_xfer);
@@ -1156,7 +1163,7 @@ pci_xhci_cmd_reset_ep(struct pci_xhci_softc *sc, uint32_t slot,
 
 	ep_ctx->dwEpCtx0 = (ep_ctx->dwEpCtx0 & ~0x7) | XHCI_ST_EPCTX_STOPPED;
 
-	if (XHCI_EPCTX_0_MAXP_STREAMS_GET(ep_ctx->dwEpCtx0) == 0)
+	if (devep->ep_MaxPStreams == 0)
 		ep_ctx->qwEpCtx2 = devep->ep_ringaddr | devep->ep_ccs;
 
 	DPRINTF(("pci_xhci: reset ep[%u] %08x %08x %016lx %08x",
@@ -1177,16 +1184,15 @@ done:
 
 static uint32_t
 pci_xhci_find_stream(struct pci_xhci_softc *sc, struct xhci_endp_ctx *ep,
-    uint32_t streamid, struct xhci_stream_ctx **osctx)
+    struct pci_xhci_dev_ep *devep, uint32_t streamid,
+    struct xhci_stream_ctx **osctx)
 {
 	struct xhci_stream_ctx *sctx;
-	uint32_t	maxpstreams;
 
-	maxpstreams = XHCI_EPCTX_0_MAXP_STREAMS_GET(ep->dwEpCtx0);
-	if (maxpstreams == 0)
+	if (devep->ep_MaxPStreams == 0)
 		return (XHCI_TRB_ERROR_TRB);
 
-	if (maxpstreams > XHCI_STREAMS_MAX)
+	if (devep->ep_MaxPStreams > XHCI_STREAMS_MAX)
 		return (XHCI_TRB_ERROR_INVALID_SID);
 
 	if (XHCI_EPCTX_0_LSA_GET(ep->dwEpCtx0) == 0) {
@@ -1195,7 +1201,7 @@ pci_xhci_find_stream(struct pci_xhci_softc *sc, struct xhci_endp_ctx *ep,
 	}
 
 	/* only support primary stream */
-	if (streamid > maxpstreams)
+	if (streamid > devep->ep_MaxPStreams)
 		return (XHCI_TRB_ERROR_STREAM_TYPE);
 
 	sctx = XHCI_GADDR(sc, ep->qwEpCtx2 & ~0xFUL) + streamid;
@@ -1257,11 +1263,12 @@ pci_xhci_cmd_set_tr(struct pci_xhci_softc *sc, uint32_t slot,
 	}
 
 	streamid = XHCI_TRB_2_STREAM_GET(trb->dwTrb2);
-	if (XHCI_EPCTX_0_MAXP_STREAMS_GET(ep_ctx->dwEpCtx0) > 0) {
+	if (devep->ep_MaxPStreams > 0) {
 		struct xhci_stream_ctx *sctx;
 
 		sctx = NULL;
-		cmderr = pci_xhci_find_stream(sc, ep_ctx, streamid, &sctx);
+		cmderr = pci_xhci_find_stream(sc, ep_ctx, devep, streamid,
+		    &sctx);
 		if (sctx != NULL) {
 			assert(devep->ep_sctx != NULL);
 
@@ -1631,7 +1638,7 @@ pci_xhci_update_ep_ring(struct pci_xhci_softc *sc, struct pci_xhci_dev_emu *dev,
     uint32_t streamid, uint64_t ringaddr, int ccs)
 {
 
-	if (XHCI_EPCTX_0_MAXP_STREAMS_GET(ep_ctx->dwEpCtx0) != 0) {
+	if (devep->ep_MaxPStreams != 0) {
 		devep->ep_sctx[streamid].qwSctx0 = (ringaddr & ~0xFUL) |
 		                                   (ccs & 0x1);
 
@@ -1942,7 +1949,7 @@ pci_xhci_device_doorbell(struct pci_xhci_softc *sc, uint32_t slot,
 	}
 
 	/* get next trb work item */
-	if (XHCI_EPCTX_0_MAXP_STREAMS_GET(ep_ctx->dwEpCtx0) != 0) {
+	if (devep->ep_MaxPStreams != 0) {
 		struct xhci_stream_ctx *sctx;
 
 		/*
@@ -1955,7 +1962,7 @@ pci_xhci_device_doorbell(struct pci_xhci_softc *sc, uint32_t slot,
 		}
 
 		sctx = NULL;
-		pci_xhci_find_stream(sc, ep_ctx, streamid, &sctx);
+		pci_xhci_find_stream(sc, ep_ctx, devep, streamid, &sctx);
 		if (sctx == NULL) {
 			DPRINTF(("pci_xhci: invalid stream %u", streamid));
 			return;