[xhci] Leak memory if controller fails to disable slot

If the Disable Slot command fails then the hardware may continue to
write to the slot context.  Leak the memory used by the slot context
to avoid future memory corruption.

This situation has been observed in practice when a Set Address
command fails, causing the command ring to become temporarily
unresponsive.

Note that there is no need to similarly leak memory on the failure
path in xhci_device_open(), since in the event of a failure the
hardware is never informed of the slot context address.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 18 insertions, 3 deletions

diff --git a/src/drivers/usb/xhci.c b/src/drivers/usb/xhci.c
index 396d943d..3574192d 100644
--- a/src/drivers/usb/xhci.c
+++ b/src/drivers/usb/xhci.c
@@ -2587,13 +2587,28 @@ static void xhci_device_close ( struct usb_device *usb ) {
 	struct xhci_device *xhci = slot->xhci;
 	size_t len = xhci_device_context_offset ( xhci, XHCI_CTX_END );
 	unsigned int id = slot->id;
+	int rc;
 
 	/* Disable slot */
-	xhci_disable_slot ( xhci, id );
+	if ( ( rc = xhci_disable_slot ( xhci, id ) ) != 0 ) {
+		/* Slot is still enabled.  Leak the slot context,
+		 * since the controller may still write to this
+		 * memory, and leave the DCBAA entry intact.
+		 *
+		 * If the controller later reports that this same slot
+		 * has been re-enabled, then some assertions will be
+		 * triggered.
+		 */
+		DBGC ( xhci, "XHCI %p slot %d leaking context memory\n",
+		      xhci, slot->id );
+		slot->context = NULL;
+	}
 
 	/* Free slot */
-	free_dma ( slot->context, len );
-	xhci->dcbaa[id] = 0;
+	if ( slot->context ) {
+		free_dma ( slot->context, len );
+		xhci->dcbaa[id] = 0;
+	}
 	xhci->slot[id] = NULL;
 	free ( slot );
 }