On Mon, Jul 31, 2023 at 4:25 AM wangy <wangyzh...@163.com> wrote:
>
> From: Yang Wang <wangyzh...@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_llind...@quicinc.com>
> Cc: Ard Biesheuvel <a...@kernel.org>
>
> Signed-off-by: Yang Wang <wangyzh...@163.com>
> Acked-by: Pedro Falcato <pedro.falc...@gmail.com>
> Reviewed-by: Ran Wang <wang...@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).
--
Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107398): https://edk2.groups.io/g/devel/message/107398
Mute This Topic: https://groups.io/mt/100455239/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
