[edk2-devel] [PATCH] Silicon/Synopsys/DesignWare: DwEmacSnpDxe: Fix bug in EmacGetDmaStatus
wangy
wangyzhaoz at 163.com
Wed Jul 26 03:07:48 UTC 2023
Hi Pedro Falcato,
At 2023-07-25 16:45:01, "Pedro Falcato" <pedro.falcato at gmail.com> wrote:
>On Tue, Jul 25, 2023 at 2:10 AM <wangyzhaoz at 163.com> wrote:
>>
>> From: Yang Wang <wangyzhaoz at 163.com>
>>
>> Check EmacGetDmaStatus input parameters
>> IrqStat may be a null pointer.
>>
>> Signed-off-by: Yang Wang <wangyzhaoz at 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?
I foud in the MnpRecycleTxBuf(), it will pass NULL in 2nd parameter at code of 'Status=Snp ->GetStatus (Snp, NULL, (VOID * *)&TxBuf);'.
then in EmacGetDmaStatus(), it will not check this pointer and use directly, causig this problem (system hang). That's why I add above check.
>Also, please CC maintainers next time.
Thanks for remding.
However, I found the maintainer mailbox info in file platforms/Maintainers.txt, is not complete match to what you listed, I am a little bit confused
R: Ard Biesheuvel <ardb+tianocore at kernel.org>
M: Leif Lindholm <quic_llindhol at quicinc.com>
May I ask if the maintainer information is obtained here?
Regards,
Yang
>
>>
>> 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 at edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato at gmail.com]
>> ------------
>>
>>
>
>
>--
>Pedro
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107260): https://edk2.groups.io/g/devel/message/107260
Mute This Topic: https://groups.io/mt/100342205/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20230726/12aa676d/attachment-0001.htm>
More information about the edk2-devel-archive
mailing list