git: 29863d1effe2 - main - xhci: Rework 64-byte context support to avoid pointer abuse
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 27 Oct 2021 17:38:45 UTC
The branch main has been updated by jrtc27:
URL: https://cgit.FreeBSD.org/src/commit/?id=29863d1effe20da3cc75ae10bd52d96edafe9e59
commit 29863d1effe20da3cc75ae10bd52d96edafe9e59
Author: Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2021-10-24 18:48:46 +0000
Commit: Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2021-10-27 17:38:37 +0000
xhci: Rework 64-byte context support to avoid pointer abuse
Currently, to support 64-byte contexts, xhci_ctx_[gs]et_le(32|64) take a
pointer to the field within a 32-byte context and, if 64-byte contexts
are in use, compute where the 64-byte context field is and use that
instead by deriving a pointer from the 32-byte field pointer. This is
done by exploiting a combination of 64-byte contexts being the same
layout as their 32-byte counterparts, just with 32 bytes of padding at
the end, and that all individual contexts are either in a device
context or an input context which itself is page-aligned. By masking out
the low 4 bits (which is the offset of the field within the 32-byte
contxt) of the offset within the page, the offset of the invididual
context within the containing device/input context can be determined,
which is itself 32 times the number of preceding contexts. Thus, adding
this value to the pointer again gets 64 times the number of preceding
contexts plus the field offset, which gives the offset of the 64-byte
context plus the field offset, which is the address of the field in the
64-byte context.
However, this involves a fair amount of lying to the compiler when
constructing these intermediate pointers, and is rather difficult to
reason about. In particular, this is problematic for CHERI, where we
compile the kernel with subobject bounds enabled; that is, unless
annotated to opt out (e.g. for C struct inheritance reasons where you
need to be able to downcast, or containerof idioms), a pointer to a
member of a struct is a capability whose bounds only cover that field,
and any attempt to dereference outside those bounds will fault,
protecting against intra-object buffer overflows. Thus the pointer given
to xhci_ctx_[gs]et_le(32|64) is a capability whose bounds only cover the
field in the 32-byte context, and computing the pointer to the 64-byte
context field takes the address out of bounds, resulting in a fault when
later dereferenced.
This can be cleaned up by using a different abstraction. Instead of
doing the 32-byte to 64-byte conversion on access to the field, we can
do the conversion when getting a pointer to the context itself, and
define proper 64-byte versions of contexts in order to let the compiler
do all the necessary arithmetic rather than do it manually ourselves.
This provides a cleaner implementation, works for CHERI and may even be
slightly more performant as it avoids the need to mess with masking
pointers (which cannot in the general case be optimised by compilers to
be reused across accesses to different fields within the same context,
since it does not know that the contexts are over-aligned compared with
the C ABI requirements).
Reviewed by: hselasky
Differential Revision: https://reviews.freebsd.org/D32554
---
sys/dev/usb/controller/xhci.c | 160 +++++++++++++++---------------------------
sys/dev/usb/controller/xhci.h | 26 +++++++
2 files changed, 81 insertions(+), 105 deletions(-)
diff --git a/sys/dev/usb/controller/xhci.c b/sys/dev/usb/controller/xhci.c
index e9d15eabc20a..0b0d9a7dc7be 100644
--- a/sys/dev/usb/controller/xhci.c
+++ b/sys/dev/usb/controller/xhci.c
@@ -88,6 +88,11 @@
#define XHCI_BUS2SC(bus) \
__containerof(bus, struct xhci_softc, sc_bus)
+#define XHCI_GET_CTX(sc, which, field, ptr) \
+ ((sc)->sc_ctx_is_64_byte ? \
+ &((struct which##64 *)(ptr))->field.ctx : \
+ &((struct which *)(ptr))->field)
+
static SYSCTL_NODE(_hw_usb, OID_AUTO, xhci, CTLFLAG_RW | CTLFLAG_MPSAFE, 0,
"USB XHCI");
@@ -163,12 +168,6 @@ static usb_error_t xhci_configure_mask(struct usb_device *,
static usb_error_t xhci_cmd_evaluate_ctx(struct xhci_softc *,
uint64_t, uint8_t);
static void xhci_endpoint_doorbell(struct usb_xfer *);
-static void xhci_ctx_set_le32(struct xhci_softc *sc, volatile uint32_t *ptr, uint32_t val);
-static uint32_t xhci_ctx_get_le32(struct xhci_softc *sc, volatile uint32_t *ptr);
-static void xhci_ctx_set_le64(struct xhci_softc *sc, volatile uint64_t *ptr, uint64_t val);
-#ifdef USB_DEBUG
-static uint64_t xhci_ctx_get_le64(struct xhci_softc *sc, volatile uint64_t *ptr);
-#endif
static const struct usb_bus_methods xhci_bus_methods;
@@ -183,26 +182,26 @@ xhci_dump_trb(struct xhci_trb *trb)
}
static void
-xhci_dump_endpoint(struct xhci_softc *sc, struct xhci_endp_ctx *pep)
+xhci_dump_endpoint(struct xhci_endp_ctx *pep)
{
DPRINTFN(5, "pep = %p\n", pep);
- DPRINTFN(5, "dwEpCtx0=0x%08x\n", xhci_ctx_get_le32(sc, &pep->dwEpCtx0));
- DPRINTFN(5, "dwEpCtx1=0x%08x\n", xhci_ctx_get_le32(sc, &pep->dwEpCtx1));
- DPRINTFN(5, "qwEpCtx2=0x%016llx\n", (long long)xhci_ctx_get_le64(sc, &pep->qwEpCtx2));
- DPRINTFN(5, "dwEpCtx4=0x%08x\n", xhci_ctx_get_le32(sc, &pep->dwEpCtx4));
- DPRINTFN(5, "dwEpCtx5=0x%08x\n", xhci_ctx_get_le32(sc, &pep->dwEpCtx5));
- DPRINTFN(5, "dwEpCtx6=0x%08x\n", xhci_ctx_get_le32(sc, &pep->dwEpCtx6));
- DPRINTFN(5, "dwEpCtx7=0x%08x\n", xhci_ctx_get_le32(sc, &pep->dwEpCtx7));
+ DPRINTFN(5, "dwEpCtx0=0x%08x\n", le32toh(pep->dwEpCtx0));
+ DPRINTFN(5, "dwEpCtx1=0x%08x\n", le32toh(pep->dwEpCtx1));
+ DPRINTFN(5, "qwEpCtx2=0x%016llx\n", (long long)le64toh(pep->qwEpCtx2));
+ DPRINTFN(5, "dwEpCtx4=0x%08x\n", le32toh(pep->dwEpCtx4));
+ DPRINTFN(5, "dwEpCtx5=0x%08x\n", le32toh(pep->dwEpCtx5));
+ DPRINTFN(5, "dwEpCtx6=0x%08x\n", le32toh(pep->dwEpCtx6));
+ DPRINTFN(5, "dwEpCtx7=0x%08x\n", le32toh(pep->dwEpCtx7));
}
static void
-xhci_dump_device(struct xhci_softc *sc, struct xhci_slot_ctx *psl)
+xhci_dump_device(struct xhci_slot_ctx *psl)
{
DPRINTFN(5, "psl = %p\n", psl);
- DPRINTFN(5, "dwSctx0=0x%08x\n", xhci_ctx_get_le32(sc, &psl->dwSctx0));
- DPRINTFN(5, "dwSctx1=0x%08x\n", xhci_ctx_get_le32(sc, &psl->dwSctx1));
- DPRINTFN(5, "dwSctx2=0x%08x\n", xhci_ctx_get_le32(sc, &psl->dwSctx2));
- DPRINTFN(5, "dwSctx3=0x%08x\n", xhci_ctx_get_le32(sc, &psl->dwSctx3));
+ DPRINTFN(5, "dwSctx0=0x%08x\n", le32toh(psl->dwSctx0));
+ DPRINTFN(5, "dwSctx1=0x%08x\n", le32toh(psl->dwSctx1));
+ DPRINTFN(5, "dwSctx2=0x%08x\n", le32toh(psl->dwSctx2));
+ DPRINTFN(5, "dwSctx3=0x%08x\n", le32toh(psl->dwSctx3));
}
#endif
@@ -234,60 +233,6 @@ xhci_iterate_hw_softc(struct usb_bus *bus, usb_bus_mem_sub_cb_t *cb)
}
}
-static void
-xhci_ctx_set_le32(struct xhci_softc *sc, volatile uint32_t *ptr, uint32_t val)
-{
- if (sc->sc_ctx_is_64_byte) {
- uint32_t offset;
- /* exploit the fact that our structures are XHCI_PAGE_SIZE aligned */
- /* all contexts are initially 32-bytes */
- offset = ((uintptr_t)ptr) & ((XHCI_PAGE_SIZE - 1) & ~(31U));
- ptr = (volatile uint32_t *)(((volatile uint8_t *)ptr) + offset);
- }
- *ptr = htole32(val);
-}
-
-static uint32_t
-xhci_ctx_get_le32(struct xhci_softc *sc, volatile uint32_t *ptr)
-{
- if (sc->sc_ctx_is_64_byte) {
- uint32_t offset;
- /* exploit the fact that our structures are XHCI_PAGE_SIZE aligned */
- /* all contexts are initially 32-bytes */
- offset = ((uintptr_t)ptr) & ((XHCI_PAGE_SIZE - 1) & ~(31U));
- ptr = (volatile uint32_t *)(((volatile uint8_t *)ptr) + offset);
- }
- return (le32toh(*ptr));
-}
-
-static void
-xhci_ctx_set_le64(struct xhci_softc *sc, volatile uint64_t *ptr, uint64_t val)
-{
- if (sc->sc_ctx_is_64_byte) {
- uint32_t offset;
- /* exploit the fact that our structures are XHCI_PAGE_SIZE aligned */
- /* all contexts are initially 32-bytes */
- offset = ((uintptr_t)ptr) & ((XHCI_PAGE_SIZE - 1) & ~(31U));
- ptr = (volatile uint64_t *)(((volatile uint8_t *)ptr) + offset);
- }
- *ptr = htole64(val);
-}
-
-#ifdef USB_DEBUG
-static uint64_t
-xhci_ctx_get_le64(struct xhci_softc *sc, volatile uint64_t *ptr)
-{
- if (sc->sc_ctx_is_64_byte) {
- uint32_t offset;
- /* exploit the fact that our structures are XHCI_PAGE_SIZE aligned */
- /* all contexts are initially 32-bytes */
- offset = ((uintptr_t)ptr) & ((XHCI_PAGE_SIZE - 1) & ~(31U));
- ptr = (volatile uint64_t *)(((volatile uint8_t *)ptr) + offset);
- }
- return (le64toh(*ptr));
-}
-#endif
-
static int
xhci_reset_command_queue_locked(struct xhci_softc *sc)
{
@@ -1397,7 +1342,7 @@ xhci_set_address(struct usb_device *udev, struct mtx *mtx, uint16_t address)
struct usb_page_search buf_dev;
struct xhci_softc *sc = XHCI_BUS2SC(udev->bus);
struct xhci_hw_dev *hdev;
- struct xhci_dev_ctx *pdev;
+ struct xhci_slot_ctx *slot;
struct xhci_endpoint_ext *pepext;
uint32_t temp;
uint16_t mps;
@@ -1490,10 +1435,11 @@ xhci_set_address(struct usb_device *udev, struct mtx *mtx, uint16_t address)
/* update device address to new value */
usbd_get_page(&hdev->device_pc, 0, &buf_dev);
- pdev = buf_dev.buffer;
+ slot = XHCI_GET_CTX(sc, xhci_dev_ctx, ctx_slot,
+ buf_dev.buffer);
usb_pc_cpu_invalidate(&hdev->device_pc);
- temp = xhci_ctx_get_le32(sc, &pdev->ctx_slot.dwSctx3);
+ temp = le32toh(slot->dwSctx3);
udev->address = XHCI_SCTX_3_DEV_ADDR_GET(temp);
/* update device state to new value */
@@ -2298,7 +2244,8 @@ xhci_configure_mask(struct usb_device *udev, uint32_t mask, uint8_t drop)
{
struct xhci_softc *sc = XHCI_BUS2SC(udev->bus);
struct usb_page_search buf_inp;
- struct xhci_input_dev_ctx *pinp;
+ struct xhci_input_ctx *input;
+ struct xhci_slot_ctx *slot;
uint32_t temp;
uint8_t index;
uint8_t x;
@@ -2307,22 +2254,23 @@ xhci_configure_mask(struct usb_device *udev, uint32_t mask, uint8_t drop)
usbd_get_page(&sc->sc_hw.devs[index].input_pc, 0, &buf_inp);
- pinp = buf_inp.buffer;
+ input = XHCI_GET_CTX(sc, xhci_input_dev_ctx, ctx_input,
+ buf_inp.buffer);
+ slot = XHCI_GET_CTX(sc, xhci_input_dev_ctx, ctx_slot, buf_inp.buffer);
if (drop) {
mask &= XHCI_INCTX_NON_CTRL_MASK;
- xhci_ctx_set_le32(sc, &pinp->ctx_input.dwInCtx0, mask);
- xhci_ctx_set_le32(sc, &pinp->ctx_input.dwInCtx1, 0);
+ input->dwInCtx0 = htole32(mask);
+ input->dwInCtx1 = htole32(0);
} else {
/*
* Some hardware requires that we drop the endpoint
* context before adding it again:
*/
- xhci_ctx_set_le32(sc, &pinp->ctx_input.dwInCtx0,
- mask & XHCI_INCTX_NON_CTRL_MASK);
+ input->dwInCtx0 = htole32(mask & XHCI_INCTX_NON_CTRL_MASK);
/* Add new endpoint context */
- xhci_ctx_set_le32(sc, &pinp->ctx_input.dwInCtx1, mask);
+ input->dwInCtx1 = htole32(mask);
/* find most significant set bit */
for (x = 31; x != 1; x--) {
@@ -2340,10 +2288,10 @@ xhci_configure_mask(struct usb_device *udev, uint32_t mask, uint8_t drop)
x = sc->sc_hw.devs[index].context_num;
/* update number of contexts */
- temp = xhci_ctx_get_le32(sc, &pinp->ctx_slot.dwSctx0);
+ temp = le32toh(slot->dwSctx0);
temp &= ~XHCI_SCTX_0_CTX_NUM_SET(31);
temp |= XHCI_SCTX_0_CTX_NUM_SET(x + 1);
- xhci_ctx_set_le32(sc, &pinp->ctx_slot.dwSctx0, temp);
+ slot->dwSctx0 = htole32(temp);
}
usb_pc_cpu_flush(&sc->sc_hw.devs[index].input_pc);
return (0);
@@ -2358,7 +2306,7 @@ xhci_configure_endpoint(struct usb_device *udev,
{
struct usb_page_search buf_inp;
struct xhci_softc *sc = XHCI_BUS2SC(udev->bus);
- struct xhci_input_dev_ctx *pinp;
+ struct xhci_endp_ctx *endp;
uint64_t ring_addr = pepext->physaddr;
uint32_t temp;
uint8_t index;
@@ -2369,8 +2317,6 @@ xhci_configure_endpoint(struct usb_device *udev,
usbd_get_page(&sc->sc_hw.devs[index].input_pc, 0, &buf_inp);
- pinp = buf_inp.buffer;
-
epno = edesc->bEndpointAddress;
type = edesc->bmAttributes & UE_XFERTYPE;
@@ -2390,6 +2336,9 @@ xhci_configure_endpoint(struct usb_device *udev,
if (mult == 0)
return (USB_ERR_BAD_BUFSIZE);
+ endp = XHCI_GET_CTX(sc, xhci_input_dev_ctx, ctx_ep[epno - 1],
+ buf_inp.buffer);
+
/* store endpoint mode */
pepext->trb_ep_mode = ep_mode;
/* store bMaxPacketSize for control endpoints */
@@ -2445,7 +2394,7 @@ xhci_configure_endpoint(struct usb_device *udev,
break;
}
- xhci_ctx_set_le32(sc, &pinp->ctx_ep[epno - 1].dwEpCtx0, temp);
+ endp->dwEpCtx0 = htole32(temp);
temp =
XHCI_EPCTX_1_HID_SET(0) |
@@ -2480,8 +2429,8 @@ xhci_configure_endpoint(struct usb_device *udev,
if (epno & 1)
temp |= XHCI_EPCTX_1_EPTYPE_SET(4);
- xhci_ctx_set_le32(sc, &pinp->ctx_ep[epno - 1].dwEpCtx1, temp);
- xhci_ctx_set_le64(sc, &pinp->ctx_ep[epno - 1].qwEpCtx2, ring_addr);
+ endp->dwEpCtx1 = htole32(temp);
+ endp->qwEpCtx2 = htole64(ring_addr);
switch (edesc->bmAttributes & UE_XFERTYPE) {
case UE_INTERRUPT:
@@ -2498,10 +2447,10 @@ xhci_configure_endpoint(struct usb_device *udev,
break;
}
- xhci_ctx_set_le32(sc, &pinp->ctx_ep[epno - 1].dwEpCtx4, temp);
+ endp->dwEpCtx4 = htole32(temp);
#ifdef USB_DEBUG
- xhci_dump_endpoint(sc, &pinp->ctx_ep[epno - 1]);
+ xhci_dump_endpoint(endp);
#endif
usb_pc_cpu_flush(&sc->sc_hw.devs[index].input_pc);
@@ -2557,7 +2506,7 @@ xhci_configure_device(struct usb_device *udev)
struct xhci_softc *sc = XHCI_BUS2SC(udev->bus);
struct usb_page_search buf_inp;
struct usb_page_cache *pcinp;
- struct xhci_input_dev_ctx *pinp;
+ struct xhci_slot_ctx *slot;
struct usb_device *hubdev;
uint32_t temp;
uint32_t route;
@@ -2574,7 +2523,7 @@ xhci_configure_device(struct usb_device *udev)
usbd_get_page(pcinp, 0, &buf_inp);
- pinp = buf_inp.buffer;
+ slot = XHCI_GET_CTX(sc, xhci_input_dev_ctx, ctx_slot, buf_inp.buffer);
rh_port = 0;
route = 0;
@@ -2649,7 +2598,7 @@ xhci_configure_device(struct usb_device *udev)
if (is_hub)
temp |= XHCI_SCTX_0_HUB_SET(1);
- xhci_ctx_set_le32(sc, &pinp->ctx_slot.dwSctx0, temp);
+ slot->dwSctx0 = htole32(temp);
temp = XHCI_SCTX_1_RH_PORT_SET(rh_port);
@@ -2658,7 +2607,7 @@ xhci_configure_device(struct usb_device *udev)
sc->sc_hw.devs[index].nports);
}
- xhci_ctx_set_le32(sc, &pinp->ctx_slot.dwSctx1, temp);
+ slot->dwSctx1 = htole32(temp);
temp = XHCI_SCTX_2_IRQ_TARGET_SET(0);
@@ -2684,7 +2633,7 @@ xhci_configure_device(struct usb_device *udev)
break;
}
- xhci_ctx_set_le32(sc, &pinp->ctx_slot.dwSctx2, temp);
+ slot->dwSctx2 = htole32(temp);
/*
* These fields should be initialized to zero, according to
@@ -2693,10 +2642,10 @@ xhci_configure_device(struct usb_device *udev)
temp = XHCI_SCTX_3_DEV_ADDR_SET(0) |
XHCI_SCTX_3_SLOT_STATE_SET(0);
- xhci_ctx_set_le32(sc, &pinp->ctx_slot.dwSctx3, temp);
+ slot->dwSctx3 = htole32(temp);
#ifdef USB_DEBUG
- xhci_dump_device(sc, &pinp->ctx_slot);
+ xhci_dump_device(slot);
#endif
usb_pc_cpu_flush(pcinp);
@@ -2725,7 +2674,7 @@ xhci_alloc_device_ext(struct usb_device *udev)
pc->tag_parent = sc->sc_bus.dma_parent_tag;
if (usb_pc_alloc_mem(pc, pg, sc->sc_ctx_is_64_byte ?
- (2 * sizeof(struct xhci_dev_ctx)) :
+ sizeof(struct xhci_dev_ctx64) :
sizeof(struct xhci_dev_ctx), XHCI_PAGE_SIZE))
goto error;
@@ -2738,7 +2687,7 @@ xhci_alloc_device_ext(struct usb_device *udev)
pc->tag_parent = sc->sc_bus.dma_parent_tag;
if (usb_pc_alloc_mem(pc, pg, sc->sc_ctx_is_64_byte ?
- (2 * sizeof(struct xhci_input_dev_ctx)) :
+ sizeof(struct xhci_input_dev_ctx64) :
sizeof(struct xhci_input_dev_ctx), XHCI_PAGE_SIZE)) {
goto error;
}
@@ -3785,7 +3734,7 @@ xhci_get_endpoint_state(struct usb_device *udev, uint8_t epno)
struct xhci_softc *sc = XHCI_BUS2SC(udev->bus);
struct usb_page_search buf_dev;
struct xhci_hw_dev *hdev;
- struct xhci_dev_ctx *pdev;
+ struct xhci_endp_ctx *endp;
uint32_t temp;
MPASS(epno != 0);
@@ -3793,10 +3742,11 @@ xhci_get_endpoint_state(struct usb_device *udev, uint8_t epno)
hdev = &sc->sc_hw.devs[udev->controller_slot_id];
usbd_get_page(&hdev->device_pc, 0, &buf_dev);
- pdev = buf_dev.buffer;
+ endp = XHCI_GET_CTX(sc, xhci_dev_ctx, ctx_ep[epno - 1],
+ buf_dev.buffer);
usb_pc_cpu_invalidate(&hdev->device_pc);
- temp = xhci_ctx_get_le32(sc, &pdev->ctx_ep[epno - 1].dwEpCtx0);
+ temp = le32toh(endp->dwEpCtx0);
return (XHCI_EPCTX_0_EPSTATE_GET(temp));
}
diff --git a/sys/dev/usb/controller/xhci.h b/sys/dev/usb/controller/xhci.h
index 802207208569..ff7ec23ca5f2 100644
--- a/sys/dev/usb/controller/xhci.h
+++ b/sys/dev/usb/controller/xhci.h
@@ -111,6 +111,11 @@ struct xhci_slot_ctx {
volatile uint32_t dwSctx7;
};
+struct xhci_slot_ctx64 {
+ struct xhci_slot_ctx ctx;
+ volatile uint8_t padding[32];
+};
+
struct xhci_endp_ctx {
volatile uint32_t dwEpCtx0;
#define XHCI_EPCTX_0_EPSTATE_SET(x) ((x) & 0x7)
@@ -156,6 +161,11 @@ struct xhci_endp_ctx {
volatile uint32_t dwEpCtx7;
};
+struct xhci_endp_ctx64 {
+ struct xhci_endp_ctx ctx;
+ volatile uint8_t padding[32];
+};
+
struct xhci_input_ctx {
#define XHCI_INCTX_NON_CTRL_MASK 0xFFFFFFFCU
volatile uint32_t dwInCtx0;
@@ -170,17 +180,33 @@ struct xhci_input_ctx {
volatile uint32_t dwInCtx7;
};
+struct xhci_input_ctx64 {
+ struct xhci_input_ctx ctx;
+ volatile uint8_t padding[32];
+};
+
struct xhci_input_dev_ctx {
struct xhci_input_ctx ctx_input;
struct xhci_slot_ctx ctx_slot;
struct xhci_endp_ctx ctx_ep[XHCI_MAX_ENDPOINTS - 1];
};
+struct xhci_input_dev_ctx64 {
+ struct xhci_input_ctx64 ctx_input;
+ struct xhci_slot_ctx64 ctx_slot;
+ struct xhci_endp_ctx64 ctx_ep[XHCI_MAX_ENDPOINTS - 1];
+};
+
struct xhci_dev_ctx {
struct xhci_slot_ctx ctx_slot;
struct xhci_endp_ctx ctx_ep[XHCI_MAX_ENDPOINTS - 1];
} __aligned(XHCI_DEV_CTX_ALIGN);
+struct xhci_dev_ctx64 {
+ struct xhci_slot_ctx64 ctx_slot;
+ struct xhci_endp_ctx64 ctx_ep[XHCI_MAX_ENDPOINTS - 1];
+} __aligned(XHCI_DEV_CTX_ALIGN);
+
struct xhci_stream_ctx {
volatile uint64_t qwSctx0;
#define XHCI_SCTX_0_DCS_GET(x) ((x) & 0x1)