From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id C6BB5740037 for ; Mon, 31 Jul 2023 09:15:34 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=LnaPEzclOTs5jxgyeVTG2NLNlwHYF75l4tdq9lNUsM8=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1690794933; v=1; b=cTHDWVsvHSCLy7OBkcz7hywWf5RU3bdBSykGVPs2/AiVKY0VYc8qlOoEdT9JVoCe6+0r/rKv OajPZ6mwMhyJeZA2Q/yGsaRnycqpMzg4qI2WVblbN4BpQbyjzTR9TtzBMZk640xfIZf62kZK/dn Z49LSQzf0RYJIWW0uAxgmrUI= X-Received: by 127.0.0.2 with SMTP id MRZlYY7687511xOowVnSyVML; Mon, 31 Jul 2023 02:15:33 -0700 X-Received: from mail-ua1-f48.google.com (mail-ua1-f48.google.com [209.85.222.48]) by mx.groups.io with SMTP id smtpd.web10.3494.1690794932707868841 for ; Mon, 31 Jul 2023 02:15:32 -0700 X-Received: by mail-ua1-f48.google.com with SMTP id a1e0cc1a2514c-79702eee5a8so1249824241.1 for ; Mon, 31 Jul 2023 02:15:32 -0700 (PDT) X-Gm-Message-State: gqyaeXS2KXTkEXGSibBVxXfQx7686176AA= X-Google-Smtp-Source: APBJJlGYPJgDILLFt+UXdJjfHuOK7rqhl3F8vs45XyAfPTsGTKqpi2FAiCyuh4ByRqjSLzuPduyqj+B5cks59LQbMDQ= X-Received: by 2002:a67:cfc7:0:b0:446:de31:c78 with SMTP id h7-20020a67cfc7000000b00446de310c78mr4782143vsm.26.1690794931471; Mon, 31 Jul 2023 02:15:31 -0700 (PDT) MIME-Version: 1.0 References: <3f5a13860d7c3a15d88cbf13c9e31301ec884f9c.1690772738.git.wangyzhaoz@163.com> In-Reply-To: <3f5a13860d7c3a15d88cbf13c9e31301ec884f9c.1690772738.git.wangyzhaoz@163.com> From: "Pedro Falcato" Date: Mon, 31 Jul 2023 10:15:20 +0100 Message-ID: Subject: Re: [edk2-devel] [edk2-platforms][PATCH V3] Silicon/Synopsys/DesignWare: DwEmacSnpDxe: Fix bug in EmacGetDmaStatus To: devel@edk2.groups.io, wangyzhaoz@163.com Cc: Leif Lindholm , Ard Biesheuvel , Ran Wang Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,pedro.falcato@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=cTHDWVsv; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Mon, Jul 31, 2023 at 4:25=E2=80=AFAM wangy wrote: > > From: Yang Wang > > 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 > Cc: Ard Biesheuvel > > Signed-off-by: Yang Wang > Acked-by: Pedro Falcato > Reviewed-by: Ran Wang > --- > .../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 =3D 0; > > + if (IrqStat !=3D NULL) { > + *IrqStat =3D 0; > + } > + > DmaStatus =3D MmioRead32 (MacBaseAddress + > DW_EMAC_DMAGRP_STATUS_OFST); > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK) { > Mask |=3D DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK; > // Rx interrupt > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_RI_SET_MSK) { > - *IrqStat |=3D EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; > - Mask |=3D DW_EMAC_DMAGRP_STATUS_RI_SET_MSK; > - } else { > - *IrqStat &=3D ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; > + if (IrqStat !=3D NULL) { > + *IrqStat |=3D EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; > + Mask |=3D DW_EMAC_DMAGRP_STATUS_RI_SET_MSK; > + } > } > + > // Tx interrupt > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TI_SET_MSK) { > - *IrqStat |=3D EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; > - Mask |=3D DW_EMAC_DMAGRP_STATUS_TI_SET_MSK; > - } else { > - *IrqStat &=3D ~EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; > + if (IrqStat !=3D NULL) { > + *IrqStat |=3D EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; > + Mask |=3D DW_EMAC_DMAGRP_STATUS_TI_SET_MSK; > + } > } > + > // Tx Buffer > if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TU_SET_MSK){ > Mask |=3D 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/cbab3c40f76ee913621a9f4= afe3398b217b0d086). --=20 Pedro -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-