Reviewed-by: Siyuan Fu <siyuan...@intel.com> > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael > Kubacki > Sent: 2020年4月9日 11:02 > To: devel@edk2.groups.io > Cc: Fu, Siyuan <siyuan...@intel.com>; Maciej Rabeda > <maciej.rab...@linux.intel.com>; Wu, Jiaxin <jiaxin...@intel.com> > Subject: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Prevent invalid PCI > BAR access > > From: Michael Kubacki <michael.kuba...@microsoft.com> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1563 > > SnpDxe initializes values for MemoryBarIndex and IoBarIndex to 0 and 1 > respectively even if calls to PciIo->GetBarAttributes never return > success. > > Later, if the BAR is used to perform IO/Mem reads/writes, a potentially > non-existent BAR index may be accessed. This change initializes the values > to an invalid BAR index (PCI_MAX_BAR) so the condition can be explicitly > checked to avoid an invalid BAR access. > > Cc: Siyuan Fu <siyuan...@intel.com> > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com> > Cc: Jiaxin Wu <jiaxin...@intel.com> > Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> > --- > NetworkPkg/SnpDxe/Callback.c | 77 ++++++++++++-------- > NetworkPkg/SnpDxe/Snp.c | 5 +- > 2 files changed, 48 insertions(+), 34 deletions(-) > > diff --git a/NetworkPkg/SnpDxe/Callback.c b/NetworkPkg/SnpDxe/Callback.c > index 0c0b81fdca8e..99b7fd3ef835 100644 > --- a/NetworkPkg/SnpDxe/Callback.c > +++ b/NetworkPkg/SnpDxe/Callback.c > @@ -4,6 +4,7 @@ > stores the interface context for the NIC that snp is trying to talk. > > Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR> > +Copyright (c) Microsoft Corporation.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -115,47 +116,59 @@ SnpUndi32CallbackMemio ( > > switch (ReadOrWrite) { > case PXE_IO_READ: > - Snp->PciIo->Io.Read ( > - Snp->PciIo, > - Width, > - Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO > base address > - MemOrPortAddr, > - 1, // count > - (VOID *) (UINTN) BufferPtr > - ); > + ASSERT (Snp->IoBarIndex < PCI_MAX_BAR); > + if (Snp->IoBarIndex < PCI_MAX_BAR) { > + Snp->PciIo->Io.Read ( > + Snp->PciIo, > + Width, > + Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO > base address > + MemOrPortAddr, > + 1, // count > + (VOID *) (UINTN) BufferPtr > + ); > + } > break; > > case PXE_IO_WRITE: > - Snp->PciIo->Io.Write ( > - Snp->PciIo, > - Width, > - Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO > base address > - MemOrPortAddr, > - 1, // count > - (VOID *) (UINTN) BufferPtr > - ); > + ASSERT (Snp->IoBarIndex < PCI_MAX_BAR); > + if (Snp->IoBarIndex < PCI_MAX_BAR) { > + Snp->PciIo->Io.Write ( > + Snp->PciIo, > + Width, > + Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO > base address > + MemOrPortAddr, > + 1, // count > + (VOID *) (UINTN) BufferPtr > + ); > + } > break; > > case PXE_MEM_READ: > - Snp->PciIo->Mem.Read ( > - Snp->PciIo, > - Width, > - Snp->MemoryBarIndex, // BAR 0, Memory base address > - MemOrPortAddr, > - 1, // count > - (VOID *) (UINTN) BufferPtr > - ); > + ASSERT (Snp->MemoryBarIndex < PCI_MAX_BAR); > + if (Snp->MemoryBarIndex < PCI_MAX_BAR) { > + Snp->PciIo->Mem.Read ( > + Snp->PciIo, > + Width, > + Snp->MemoryBarIndex, // BAR 0, Memory base address > + MemOrPortAddr, > + 1, // count > + (VOID *) (UINTN) BufferPtr > + ); > + } > break; > > case PXE_MEM_WRITE: > - Snp->PciIo->Mem.Write ( > - Snp->PciIo, > - Width, > - Snp->MemoryBarIndex, // BAR 0, Memory base address > - MemOrPortAddr, > - 1, // count > - (VOID *) (UINTN) BufferPtr > - ); > + ASSERT (Snp->MemoryBarIndex < PCI_MAX_BAR); > + if (Snp->MemoryBarIndex < PCI_MAX_BAR) { > + Snp->PciIo->Mem.Write ( > + Snp->PciIo, > + Width, > + Snp->MemoryBarIndex, // BAR 0, Memory base address > + MemOrPortAddr, > + 1, // count > + (VOID *) (UINTN) BufferPtr > + ); > + } > break; > } > > diff --git a/NetworkPkg/SnpDxe/Snp.c b/NetworkPkg/SnpDxe/Snp.c > index 078b27cf5edd..da9fba51eff5 100644 > --- a/NetworkPkg/SnpDxe/Snp.c > +++ b/NetworkPkg/SnpDxe/Snp.c > @@ -2,6 +2,7 @@ > Implementation of driver entry point and driver binding protocol. > > Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR> > +Copyright (c) Microsoft Corporation.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -465,8 +466,8 @@ SimpleNetworkDriverStart ( > // the IO BAR. Save the index of the BAR into the adapter info structure. > // for regular 32bit BARs, 0 is memory mapped, 1 is io mapped > // > - Snp->MemoryBarIndex = 0; > - Snp->IoBarIndex = 1; > + Snp->MemoryBarIndex = PCI_MAX_BAR; > + Snp->IoBarIndex = PCI_MAX_BAR; > FoundMemoryBar = FALSE; > FoundIoBar = FALSE; > for (BarIndex = 0; BarIndex < PCI_MAX_BAR; BarIndex++) { > -- > 2.16.3.windows.1 > > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57485): https://edk2.groups.io/g/devel/message/57485 Mute This Topic: https://groups.io/mt/72889517/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
