From nobody Thu Aug 25 17:31:59 2022 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4MD94v4qSHz4b38h; Thu, 25 Aug 2022 17:31:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4MD94v3S9Pz3McR; Thu, 25 Aug 2022 17:31:59 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1661448719; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=HV3+9ctwt2s0aeF8fYLXzh1vB9ttWVu7HF9qJsjN/eg=; b=UTJ0fmJdb+/L9iUY20fyG5sUc/9c+DLmeAVb8/Gig0XR/uPm5UdsEC5IXirodiGgNz332O x+IqcXVzT8OzsBPfCbmge6iJtiAQdKdpzFggMIMSg4JKdgc5Di5gyeZ1KsZqyIxrMBLt7b GXXAG/fQo/Tsgh+wvBtIp6oEPM9/6gyNAYZ9KBdkRaPM1v9Wx9eD0SXe0YKt6mkbAT9xHt AMB5jNLfb/fRwRv41sofjAaWez90EoaLKbhL2hAcHJ1mlIIISsNrd3KLj0NdaJL2NJ8/om c7u5R8rAsiKG+4udyiOjYZWfSxqz2wQQe8D6YPAP80/yOzjCy2LXsOlZtkBL4A== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4MD94v2XHGzwxt; Thu, 25 Aug 2022 17:31:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 27PHVxxR043894; Thu, 25 Aug 2022 17:31:59 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 27PHVxX7043893; Thu, 25 Aug 2022 17:31:59 GMT (envelope-from git) Date: Thu, 25 Aug 2022 17:31:59 GMT Message-Id: <202208251731.27PHVxX7043893@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: John Baldwin Subject: git: 920489d03ced - stable/12 - bhyve xhci: Cache the value of MaxPStreams when initializing an endpoint. List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jhb X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 920489d03cedbe232c843f36fa5f2d954a5aae15 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1661448719; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=HV3+9ctwt2s0aeF8fYLXzh1vB9ttWVu7HF9qJsjN/eg=; b=oU5TsqoVcYNX0rfCJ5cM+PXb5LvZPZ5R3x71g4CVWGmPudqpYb4/ZBHuqE1oVkBWUDsAyf bjpvFeFWJgLFK8C3RJ6lzdgqmxd/BKtH6uTC7TnKvaoPKlzFmjfIKC1gi1+hym4sINn56E KfBBFAivsM8YBh3eI/+14sYImBvG9SciJK04qC0ojUK//lE9/SAm96DrvN0sgWAByFjZNU Yd49KeuiI0zJjbw+MFfArMFoVaO/NqyJKk3aJgLFy5/hrOwAA10Yer9zLEZcugs9ltVlJZ rDMM1s0BCiD2+D2O6VgcT+hd+pZcCAp/F9rohAMiNd2QyvYRCewNpSMuJEEtRw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1661448719; a=rsa-sha256; cv=none; b=oc5NVsbE7t+POeRQkOQbqX3lfrzqPwHY6gW+snnCLJmlUe1OM0+mbfVXFdFNmz1k2x9Ycg 4G4I418UC/WLeRHsIDqGf5AJcW6jjnVfbSlXMHCH5vYrjdPWmrqgwumDBcROEauECstVu5 lfLsexzg3WYaiir4yHjB1UR5BWS0cxxNjYzYlmilRq4qy7J9ErTT9deo6wnVNwXzQ9uFxn aoeySYQvzfm7p/gTpW/zXMWDBvdhpa3Gz8bURLdHXjYDqBTChfUGSo4uFBLT6AKq3Pb0Bx HiwICe5UvvHtsk65wSEtl3pkoBE8wYzfTBo4o0YMfvp5AxRtW5DaguLNixd1mg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/12 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=920489d03cedbe232c843f36fa5f2d954a5aae15 commit 920489d03cedbe232c843f36fa5f2d954a5aae15 Author: John Baldwin AuthorDate: 2022-08-17 17:00:36 +0000 Commit: John Baldwin CommitDate: 2022-08-25 16:39:22 +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 Reviewed by: markj MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D36181 (cherry picked from commit e7439f6aeb235ba3a7e79818c56a63d066c80854) --- 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 beca9181871e..c76bcc3c4be6 100644 --- a/usr.sbin/bhyve/pci_xhci.c +++ b/usr.sbin/bhyve/pci_xhci.c @@ -165,6 +165,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; @@ -667,6 +674,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)); @@ -688,9 +696,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); @@ -1149,7 +1156,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", @@ -1170,16 +1177,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) { @@ -1188,7 +1194,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; @@ -1250,11 +1256,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); @@ -1624,7 +1631,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); @@ -1935,7 +1942,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; /* @@ -1948,7 +1955,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;