[efi] Leave asynchronous USB endpoints open until device is removed

Some UEFI device drivers will react to an asynchronous USB transfer
failure by dubiously terminating the scheduled transfer from within
the completion handler.

We already have code from commit fbb776f ("[efi] Leave USB endpoint
descriptors in existence until device is removed") that avoids freeing
memory in this situation, in order to avoid use-after-free bugs.  This
is not sufficient to avoid potential problems, since with an xHCI
controller the act of closing the endpoint requires issuing a command
and awaiting completion via the event ring, which may in turn dispatch
further USB transfer completion events.

Avoid these problems by leaving the USB endpoint open (but with the
refill timer stopped) until the device is finally removed, as is
already done for control and bulk transfers.

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

diff --git a/src/interface/efi/efi_usb.c b/src/interface/efi/efi_usb.c
index df66df45f1b5e3d1e401e422b7800ef5b81cd14b..28dfc8680c4762ddbcd6b7406347ce2ce5bf699a 100644 (file)
--- a/src/interface/efi/efi_usb.c
+++ b/src/interface/efi/efi_usb.c
@@ -415,9 +415,11 @@ static void efi_usb_async_complete ( struct usb_endpoint *ep,
        /* Construct status */
        status = ( ( rc == 0 ) ? 0 : EFI_USB_ERR_SYSTEM );
 
-       /* Report completion */
-       usbep->callback ( iobuf->data, iob_len ( iobuf ), usbep->context,
-                         status );
+       /* Report completion, if applicable */
+       if ( usbep->callback ) {
+               usbep->callback ( iobuf->data, iob_len ( iobuf ),
+                                 usbep->context, status );
+       }
 
  drop:
        /* Recycle or free I/O buffer */
@@ -456,11 +458,9 @@ static int efi_usb_async_start ( struct efi_usb_interface *usbintf,
        EFI_STATUS efirc;
        int rc;
 
-       /* Fail if endpoint is already open */
-       if ( efi_usb_is_open ( usbintf, endpoint ) ) {
-               rc = -EINVAL;
-               goto err_already_open;
-       }
+       /* Close endpoint, if applicable */
+       if ( efi_usb_is_open ( usbintf, endpoint ) )
+               efi_usb_close ( usbintf->endpoint[index] );
 
        /* Open endpoint */
        if ( ( rc = efi_usb_open ( usbintf, endpoint,
@@ -497,9 +497,10 @@ static int efi_usb_async_start ( struct efi_usb_interface *usbintf,
        bs->SetTimer ( usbep->event, TimerCancel, 0 );
  err_timer:
  err_prefill:
+       usbep->callback = NULL;
+       usbep->context = NULL;
        efi_usb_close ( usbep );
  err_open:
- err_already_open:
        return rc;
 }
 
@@ -523,8 +524,9 @@ static void efi_usb_async_stop ( struct efi_usb_interface *usbintf,
        /* Stop timer */
        bs->SetTimer ( usbep->event, TimerCancel, 0 );
 
-       /* Close endpoint */
-       efi_usb_close ( usbep );
+       /* Clear callback parameters */
+       usbep->callback = NULL;
+       usbep->context = NULL;
 }
 
 /******************************************************************************