Mike, I do not insist on the check that the Direction is not EfiUsbNoData, and I know the functionality should have no improvement with the check. So, you can have my Reviewed-by: Star Zeng <[email protected]>.
I raised the question just for consideration from literally, as according to the spec, the code could not touch DataLength at all when Direction is EfiUsbNoData. You can decide to have / have not the check when pushing. :) Thanks, Star -----Original Message----- From: Kinney, Michael D Sent: Wednesday, November 15, 2017 10:46 AM To: Zeng, Star <[email protected]>; [email protected]; Kinney, Michael D <[email protected]> Cc: Dong, Eric <[email protected]> Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add UsbControlTransfer() error check Star, For that case both DataLength and RequestedDataLength will be 0 and the new path will not be taken. Are you concerned that the USB HC Protocol could return a non zero DataLength for the EfiUsbNoData case? Even if it does, the new path can never be taken because 0 is less than all UINTN values. Mike > -----Original Message----- > From: Zeng, Star > Sent: Tuesday, November 14, 2017 6:40 PM > To: Kinney, Michael D <[email protected]>; > [email protected] > Cc: Dong, Eric <[email protected]>; Zeng, Star <[email protected]> > Subject: RE: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add > UsbControlTransfer() error check > > Mike, > > Do you think it is better or not to also check the Direction is not > EfiUsbNoData? > > UEFI spec, EFI_USB_IO_PROTOCOL.UsbControlTransfer(): > If the Direction parameter is EfiUsbNoData, Data is NULL, and > DataLength is 0, then no data phase exists for the control transfer. > > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Wednesday, November 15, 2017 9:03 AM > To: [email protected] > Cc: Zeng, Star <[email protected]>; Dong, Eric <[email protected]> > Subject: [Patch V2 1/2] MdeModulePkg/UsbBusDxe: Add > UsbControlTransfer() error check > > https://bugzilla.tianocore.org/show_bug.cgi?id=767 > > The USB I/O Protocol function ControlTransfer() has a DataLength > parameter that specifies the size of the Data buffer. The UsbBusDxe > module implements the USB I/O Protocol using the services of the USB2 > Host Controller Protocol. The DataLength parameter in the > USB2 Host Controller Protocol ControlTransfer() service is an IN OUT > parameter so the number of bytes actually transferred is returned. > Since the USB I/O Protocol > ControlTransfer() service can not return the number of bytes actually > transferred, the only option if the number of bytes actually > transferred is less than the number of bytes requested is to return > EFI_DEVICE_ERROR. > > The change fixes an issue with a USB mass storage device that responds > with 0 bytes to the Get MAX LUN command. > > Cc: Star Zeng <[email protected]> > Cc: Eric Dong <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Michael D Kinney > <[email protected]> > --- > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > index 78220222f6..b0ad7938e9 100644 > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > @@ -2,7 +2,7 @@ > > Usb Bus Driver Binding and Bus IO Protocol. > > -Copyright (c) 2004 - 2016, Intel Corporation. All rights > reserved.<BR> > +Copyright (c) 2004 - 2017, Intel Corporation. All > rights reserved.<BR> > This program and the accompanying materials are licensed and made > available under the terms and conditions of the BSD License which > accompanies this distribution. The full text of the license may be > found at @@ -76,6 +76,7 @@ UsbIoControlTransfer ( > USB_ENDPOINT_DESC *EpDesc; > EFI_TPL OldTpl; > EFI_STATUS Status; > + UINTN RequestedDataLength; > > if (UsbStatus == NULL) { > return EFI_INVALID_PARAMETER; > @@ -86,6 +87,7 @@ UsbIoControlTransfer ( > UsbIf = USB_INTERFACE_FROM_USBIO (This); > Dev = UsbIf->Device; > > + RequestedDataLength = DataLength; > Status = UsbHcControlTransfer ( > Dev->Bus, > Dev->Address, > @@ -99,6 +101,10 @@ UsbIoControlTransfer ( > &Dev->Translator, > UsbStatus > ); > + if (!EFI_ERROR (Status) && DataLength < > RequestedDataLength) { > + Status = EFI_DEVICE_ERROR; > + goto ON_EXIT; > + } > > if (EFI_ERROR (Status) || (*UsbStatus != > EFI_USB_NOERROR)) { > // > -- > 2.14.2.windows.3 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
