The second union in Bhyve's struct pci_xhci_dev_ep causes these two fields to occupy the same memory: #define ep_ringaddr _ep_trb_rings._epu_trb.ringaddr #define ep_sctx_trbs _ep_trb_rings._epu_sctx_trbs Which of the two bhyve's pci_xhci.c uses depends on whether the guest configures the number of streams to be non-zero. Bhyve interprets one as a guest address that must be checked and mapped before each use, and the other as a host address that can be used directly with no checking. In the former case, the guest provides the value; in the latter, Bhyve. The number of streams is determined by bits in ep_ctx->dwEpCtx0, which lives in guest memory. So it's possible for the guest to tell byhve's xhci to configure an endpoint with the number of streams set to zero, which causes pci_xhci_init_ep() to set devep->ep_ringaddr to a guest-provided value XYZ (assumed to be an address in guest memory, but can be anything the guest wants). Later, the guest can change ep_ctx->dwEpCtx0 to indicate multiple streams, and write to an xhci doorbell register, causing Byhyve's pci_xhci_device_doorbell() to execute: sctx_tr = &devep->ep_sctx_trbs[streamid]; ringaddr = sctx_tr->ringaddr; which interprets devep->ep_sctx_trbs as a *host* pointer. But those bits contain the guest-provided value XYZ previously written there by pci_xhci_init_ep(). So "sctx_tr->ringaddr" will dereference a pointer provided by the guest, without any checks. I have a modified FreeBSD guest that makes Bhyve crash due to this bug.
I have written a patch for both this and bug 26347 that is available at https://reviews.freebsd.org/D36181 I have not written my own PoC for either of these issues yet. Are you able to easily verify possible fixes on your end?
(In reply to John Baldwin from comment #1) Your patch D36181 does indeed make this bug go away for me.
A commit in branch main references this bug: 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(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ecfbefc91e75674899b3014ab39720f4a13beca5 commit ecfbefc91e75674899b3014ab39720f4a13beca5 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-08-17 17:00:36 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-08-25 16:35:12 +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 (cherry picked from commit e7439f6aeb235ba3a7e79818c56a63d066c80854) usr.sbin/bhyve/pci_xhci.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=920489d03cedbe232c843f36fa5f2d954a5aae15 commit 920489d03cedbe232c843f36fa5f2d954a5aae15 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-08-17 17:00:36 +0000 Commit: John Baldwin <jhb@FreeBSD.org> 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 <rtm@lcs.mit.edu> 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(-)