Re: [PATCH 05/11] MdeModulePkg/Usb: Make sure data from HW is no more than expected

["Zeng, Star" <star.zeng@intel.com>]

On 2018/10/15 14:38, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>

Ray,

Thanks for the patch.
Reviewed-by: Star Zeng <star.zeng@intel.com>

Star

> ---
>   MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 9 ++++++---
>   MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c | 7 ++++---
>   MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 9 ++++++---
>   3 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> index fea6f47f4c..168280be81 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> @@ -1009,9 +1009,12 @@ EhcMonitorAsyncRequests (
>       ProcBuf = NULL;
>   
>       if (Urb->Result == EFI_USB_NOERROR) {
> -      ASSERT (Urb->Completed <= Urb->DataLen);
> -
> -      ProcBuf = AllocatePool (Urb->Completed);
> +      //
> +      // Make sure the data received from HW is no more than expected.
> +      //
> +      if (Urb->Completed <= Urb->DataLen) {
> +        ProcBuf = AllocatePool (Urb->Completed);
> +      }
>   
>         if (ProcBuf == NULL) {
>           EhcUpdateAsyncRequest (Ehc, Urb);
> diff --git a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
> index 90f010c998..f7510f3ec0 100644
> --- a/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/UhciDxe/UhciSched.c
> @@ -2,7 +2,7 @@
>   
>     The EHCI register operation routines.
>   
> -Copyright (c) 2007 - 2013, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2007 - 2018, 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
> @@ -1001,11 +1001,12 @@ UhciMonitorAsyncReqList (
>   
>       //
>       // Copy the data to temporary buffer if there are some
> -    // data transferred. We may have zero-length packet
> +    // data transferred. We may have zero-length packet.
> +    // Make sure the data received from HW is no more than expected.
>       //
>       Data = NULL;
>   
> -    if (QhResult.Complete != 0) {
> +    if ((QhResult.Complete != 0) && (QhResult.Complete <= AsyncReq->DataLen)) {
>         Data = AllocatePool (QhResult.Complete);
>   
>         if (Data == NULL) {
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 6a2ef4cd5d..166c44bf5e 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -1556,9 +1556,12 @@ XhcMonitorAsyncRequests (
>       //
>       ProcBuf = NULL;
>       if (Urb->Result == EFI_USB_NOERROR) {
> -      ASSERT (Urb->Completed <= Urb->DataLen);
> -
> -      ProcBuf = AllocateZeroPool (Urb->Completed);
> +      //
> +      // Make sure the data received from HW is no more than expected.
> +      //
> +      if (Urb->Completed <= Urb->DataLen) {
> +        ProcBuf = AllocateZeroPool (Urb->Completed);
> +      }
>   
>         if (ProcBuf == NULL) {
>           XhcUpdateAsyncRequest (Xhc, Urb);
>