<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div style="margin: 0;"><span style="font-family: arial; white-space: pre-wrap;">Hi Pedro Falcato,</span></div><div style="margin: 0;"><span style="font-family: arial; white-space: pre-wrap;"><br></span></div><div style="margin: 0;"><span style="font-family: arial; white-space: pre-wrap;">At 2023-07-25 16:45:01, "Pedro Falcato" <pedro.falcato@gmail.com> wrote:</span></div><pre>>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
<div>>error out if it isn't provided?</div><div><br></div><div>I foud in the MnpRecycleTxBuf(), it will pass NULL in 2nd parameter at code of 'Status=Snp ->GetStatus (Snp, NULL, (VOID * *)&TxBuf);'.</div><div>then in EmacGetDmaStatus(), it will not check this pointer and use directly, causig this problem (system hang). That's why I add above check.</div><div><br></div><div>>Also, please CC maintainers next time.</div><div><br></div><div>Thanks for remding.</div><div><br></div><div>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<br></div><div>R: Ard Biesheuvel <ardb+tianocore@kernel.org>
M: Leif Lindholm <quic_llindhol@quicinc.com></div><div>May I ask if the maintainer information is obtained here?</div><div><br></div><div>Regards,</div><div><br></div><div>Yang</div><div><br></div><div>></div>>>
>> 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
>
>
>
>
</pre></div>
<div width="1" style="color:white;clear:both">_._,_._,_</div>
<hr>
Groups.io Links:<p>
You receive all messages sent to this group.
<p>
<a target="_blank" href="https://edk2.groups.io/g/devel/message/107260">View/Reply Online (#107260)</a> |
|
<a target="_blank" href="https://groups.io/mt/100342205/1813853">Mute This Topic</a>
| <a href="https://edk2.groups.io/g/devel/post">New Topic</a>
<br>
<a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> |
<a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |
<a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a>
[edk2-devel-archive@redhat.com]<br>
<div width="1" style="color:white;clear:both">_._,_._,_</div>