Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Avoid buggy USB short-form expanding

["Fan, Jeff" <jeff.fan@intel.com>]

Reviewed-by: Jeff Fan <jeff.fan@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Ruiyu Ni
> Sent: Wednesday, April 19, 2017 5:17 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Michael Turner 
> <Michael.Turner@microsoft.com>; Fan, Jeff <jeff.fan@intel.com>; Dong, 
> Eric <eric.dong@intel.com>
> Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Avoid buggy 
> USB short-form expanding
> 
> When a load option points to a physical UsbIo controller, whose device 
> path contains UsbClass or UsbWwid node, old logic unconditionally 
> treats it as a short-form device path and expands it. But the 
> expanding gets the exactly same device path, and the device path is 
> passed to
> BmGetNextLoadOptionDevicePath() which then passes this device path to
> BmExpandUsbDevicePath() again.
> This causes a infinite recursion.
> 
> The patch avoids the USB short-form expanding when the device path 
> points to a physical UsbIo controller.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 43
> ++++++++++++++----------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index aa79c90..d684482 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1546,26 +1546,35 @@ BmGetNextLoadOptionDevicePath (
>      //
>      return BmExpandUriDevicePath (FilePath, FullPath);
>    } else {
> -    for (Node = FilePath; !IsDevicePathEnd (Node); Node =
> NextDevicePathNode (Node)) {
> -      if ((DevicePathType (Node) == MESSAGING_DEVICE_PATH) &&
> -          ((DevicePathSubType (Node) == MSG_USB_CLASS_DP) ||
> (DevicePathSubType (Node) == MSG_USB_WWID_DP))) {
> -        break;
> +    Node = FilePath;
> +    Status = gBS->LocateDevicePath (&gEfiUsbIoProtocolGuid, &Node,
> &Handle);
> +    if (EFI_ERROR (Status)) {
> +      //
> +      // Only expand the USB WWID/Class device path
> +      // when FilePath doesn't point to a physical UsbIo controller.
> +      // Otherwise, infinite recursion will happen.
> +      //
> +      for (Node = FilePath; !IsDevicePathEnd (Node); Node =
> NextDevicePathNode (Node)) {
> +        if ((DevicePathType (Node) == MESSAGING_DEVICE_PATH) &&
> +            ((DevicePathSubType (Node) == MSG_USB_CLASS_DP) ||
> (DevicePathSubType (Node) == MSG_USB_WWID_DP))) {
> +          break;
> +        }
>        }
> -    }
> 
> -    //
> -    // Expand the USB WWID/Class device path
> -    //
> -    if (!IsDevicePathEnd (Node)) {
> -      if (FilePath == Node) {
> -        //
> -        // Boot Option device path starts with USB Class or USB WWID device
> path.
> -        // For Boot Option device path which doesn't begin with the USB Class
> or
> -        // USB WWID device path, it's not needed to connect again here.
> -        //
> -        BmConnectUsbShortFormDevicePath (FilePath);
> +      //
> +      // Expand the USB WWID/Class device path
> +      //
> +      if (!IsDevicePathEnd (Node)) {
> +        if (FilePath == Node) {
> +          //
> +          // Boot Option device path starts with USB Class or USB 
> + WWID device
> path.
> +          // For Boot Option device path which doesn't begin with the 
> + USB Class
> or
> +          // USB WWID device path, it's not needed to connect again here.
> +          //
> +          BmConnectUsbShortFormDevicePath (FilePath);
> +        }
> +        return BmExpandUsbDevicePath (FilePath, FullPath, Node);
>        }
> -      return BmExpandUsbDevicePath (FilePath, FullPath, Node);
>      }
>    }
> 
> --
> 2.9.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel