Re: [edk2-devel] [PATCH] Silicon/Synopsys/DesignWare: DwEmacSnpDxe: Fix bug in EmacGetDmaStatus

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Tue, Jul 25, 2023 at 2:10 AM <wangyzhaoz@163.com> wrote:
>
> From: Yang Wang <wangyzhaoz@163.com>
>
> Check EmacGetDmaStatus input parameters
> IrqStat may be a null pointer.
>
> Signed-off-by: Yang Wang <wangyzhaoz@163.com>
> ---
>  .../Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c          |  7 +++++--
>  .../Drivers/DwEmacSnpDxe/EmacDxeUtil.c           | 16 ++++++++++++----
>  .../Drivers/DwEmacSnpDxe/EmacDxeUtil.h           |  2 +-
>  3 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c
> index 4cb3371d79..6805511a1d 100755
> --- a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c
> +++ b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c
> @@ -847,9 +847,12 @@ SnpGetStatus (
>    }
>
>    // Check DMA Irq status
> -  EmacGetDmaStatus (IrqStat, Snp->MacBase);
> +  Status = EmacGetDmaStatus (IrqStat, Snp->MacBase);
> +  if (EFI_ERROR(Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: error  Status: %r\n", __func__, Status));
> +  }
>
> -  return EFI_SUCCESS;
> +  return Status;
>  }
>
>
> diff --git a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
> index 3b982ce984..45b5a05f51 100755
> --- a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
> +++ b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
> @@ -489,16 +489,22 @@ EmacDmaStart (
>  }
>
>
> -VOID
> +EFI_STATUS
>  EFIAPI
>  EmacGetDmaStatus (
>    OUT  UINT32   *IrqStat  OPTIONAL,
>    IN   UINTN    MacBaseAddress
>    )
>  {
> -  UINT32  DmaStatus;
> -  UINT32  ErrorBit;
> -  UINT32  Mask = 0;
> +  UINT32        DmaStatus;
> +  UINT32        ErrorBit;
> +  UINT32        Mask = 0;
> +  EFI_STATUS    Status = EFI_SUCCESS;
> +
> +  if (IrqStat == NULL) {
> +    Status = EFI_INVALID_PARAMETER;
> +    goto EXIT;
> +  }

This patch looks bogus to me. IrqStat is marked OPTIONAL, how can you
error out if it isn't provided?
Also, please CC maintainers next time.

>
>    DmaStatus = MmioRead32 (MacBaseAddress +
>                             DW_EMAC_DMAGRP_STATUS_OFST);
> @@ -602,6 +608,8 @@ EmacGetDmaStatus (
>    MmioOr32 (MacBaseAddress +
>              DW_EMAC_DMAGRP_STATUS_OFST,
>              Mask);
> +EXIT:
> +  return Status;
>  }
>
>
> diff --git a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h
> index c4c3653dc7..60f30ecd16 100755
> --- a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h
> +++ b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h
> @@ -339,7 +339,7 @@ EmacDmaStart (
>    );
>
>
> -VOID
> +EFI_STATUS
>  EFIAPI
>  EmacGetDmaStatus (
>    OUT UINT32                  *IrqStat  OPTIONAL,
> --
> 2.25.1
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#107195): https://edk2.groups.io/g/devel/message/107195
> Mute This Topic: https://groups.io/mt/100342205/5946980
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato@gmail.com]
> ------------
>
>


-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107226): https://edk2.groups.io/g/devel/message/107226
Mute This Topic: https://groups.io/mt/100342205/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-