[edk2-devel] [edk2-platforms][PATCH V3] Silicon/Synopsys/DesignWare: DwEmacSnpDxe: Fix bug in EmacGetDmaStatus
wangy
wangyzhaoz at 163.com
Tue Aug 1 02:23:11 UTC 2023
Hi Pedro Falcato,
At 2023-07-31 17:15:20, "Pedro Falcato" <pedro.falcato at gmail.com> wrote:
>On Mon, Jul 31, 2023 at 4:25 AM wangy <wangyzhaoz at 163.com> wrote:
>>
>> From: Yang Wang <wangyzhaoz at 163.com>
>>
>> The EFI spec (see UEFI 2.10, 24.1.12) requires
>> EFI_SIMPLE_NETWORK.GetStatus() to handle NULL InterruptStatus pointers
>> by not reading nor clearing the interrupt status from the device.
>>
>> However, EmacGetDmaStatus (part of the DwEmacSnpDxe GetStatus()
>> implementation) did not correctly handle NULL IrqStat, despite already
>> being tagged as an OPTIONAL argument. This made calling GetStatus()
>> with a NULL pointer (for example, the call in MnpRecycleTxBuf) either
>> corrupt memory or straight-up crash.
>>
>> Make it EFI spec compliant, by adding proper NULL pointer checks
>> around RI_SET_MSK and TI_SET_MSK retrieval/clearing.
>>
>> Cc: Leif Lindholm <quic_llindhol at quicinc.com>
>> Cc: Ard Biesheuvel <ardb at kernel.org>
>>
>> Signed-off-by: Yang Wang <wangyzhaoz at 163.com>
>> Acked-by: Pedro Falcato <pedro.falcato at gmail.com>
>> Reviewed-by: Ran Wang <wangran at bosc.ac.cn>
>> ---
>> .../Drivers/DwEmacSnpDxe/EmacDxeUtil.c | 22 ++++++++++++-------
>> 1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
>> index 3b982ce984..26d3ff6138 100755
>> --- a/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
>> +++ b/Silicon/Synopsys/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
>> @@ -500,24 +500,30 @@ EmacGetDmaStatus (
>> UINT32 ErrorBit;
>> UINT32 Mask = 0;
>>
>> + if (IrqStat != NULL) {
>> + *IrqStat = 0;
>> + }
>> +
>> DmaStatus = MmioRead32 (MacBaseAddress +
>> DW_EMAC_DMAGRP_STATUS_OFST);
>> if (DmaStatus & DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK) {
>> Mask |= DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK;
>> // Rx interrupt
>> if (DmaStatus & DW_EMAC_DMAGRP_STATUS_RI_SET_MSK) {
>> - *IrqStat |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
>> - Mask |= DW_EMAC_DMAGRP_STATUS_RI_SET_MSK;
>> - } else {
>> - *IrqStat &= ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
>> + if (IrqStat != NULL) {
>> + *IrqStat |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
>> + Mask |= DW_EMAC_DMAGRP_STATUS_RI_SET_MSK;
>> + }
>> }
>> +
>> // Tx interrupt
>> if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TI_SET_MSK) {
>> - *IrqStat |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
>> - Mask |= DW_EMAC_DMAGRP_STATUS_TI_SET_MSK;
>> - } else {
>> - *IrqStat &= ~EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
>> + if (IrqStat != NULL) {
>> + *IrqStat |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
>> + Mask |= DW_EMAC_DMAGRP_STATUS_TI_SET_MSK;
>> + }
>> }
>> +
>> // Tx Buffer
>> if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TU_SET_MSK){
>> Mask |= DW_EMAC_DMAGRP_STATUS_TU_SET_MSK;
>> --
>> 2.25.1
>
>Hi,
>
>No need for a v3, Ard has already applied the patch
>(https://github.com/tianocore/edk2-platforms/commit/cbab3c40f76ee913621a9f4afe3398b217b0d086).
Thank you very much, I see.
Regards,
Yang
> >-- >Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107416): https://edk2.groups.io/g/devel/message/107416
Mute This Topic: https://groups.io/mt/100455239/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/20230801/88436265/attachment-0001.htm>
More information about the edk2-devel-archive
mailing list