NetworkPkg/SnpDxe: Prevent invalid PCI BAR access

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.fu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
This commit is contained in:
Michael Kubacki 2020-04-08 20:02:05 -07:00 committed by mergify[bot]
parent ca08f3d453
commit df4f154da9
2 changed files with 47 additions and 34 deletions

View File

@ -4,6 +4,7 @@
stores the interface context for the NIC that snp is trying to talk. 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) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent SPDX-License-Identifier: BSD-2-Clause-Patent
**/ **/
@ -115,47 +116,59 @@ SnpUndi32CallbackMemio (
switch (ReadOrWrite) { switch (ReadOrWrite) {
case PXE_IO_READ: case PXE_IO_READ:
Snp->PciIo->Io.Read ( ASSERT (Snp->IoBarIndex < PCI_MAX_BAR);
Snp->PciIo, if (Snp->IoBarIndex < PCI_MAX_BAR) {
Width, Snp->PciIo->Io.Read (
Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO base address Snp->PciIo,
MemOrPortAddr, Width,
1, // count Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO base address
(VOID *) (UINTN) BufferPtr MemOrPortAddr,
); 1, // count
(VOID *) (UINTN) BufferPtr
);
}
break; break;
case PXE_IO_WRITE: case PXE_IO_WRITE:
Snp->PciIo->Io.Write ( ASSERT (Snp->IoBarIndex < PCI_MAX_BAR);
Snp->PciIo, if (Snp->IoBarIndex < PCI_MAX_BAR) {
Width, Snp->PciIo->Io.Write (
Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO base address Snp->PciIo,
MemOrPortAddr, Width,
1, // count Snp->IoBarIndex, // BAR 1 (for 32bit regs), IO base address
(VOID *) (UINTN) BufferPtr MemOrPortAddr,
); 1, // count
(VOID *) (UINTN) BufferPtr
);
}
break; break;
case PXE_MEM_READ: case PXE_MEM_READ:
Snp->PciIo->Mem.Read ( ASSERT (Snp->MemoryBarIndex < PCI_MAX_BAR);
Snp->PciIo, if (Snp->MemoryBarIndex < PCI_MAX_BAR) {
Width, Snp->PciIo->Mem.Read (
Snp->MemoryBarIndex, // BAR 0, Memory base address Snp->PciIo,
MemOrPortAddr, Width,
1, // count Snp->MemoryBarIndex, // BAR 0, Memory base address
(VOID *) (UINTN) BufferPtr MemOrPortAddr,
); 1, // count
(VOID *) (UINTN) BufferPtr
);
}
break; break;
case PXE_MEM_WRITE: case PXE_MEM_WRITE:
Snp->PciIo->Mem.Write ( ASSERT (Snp->MemoryBarIndex < PCI_MAX_BAR);
Snp->PciIo, if (Snp->MemoryBarIndex < PCI_MAX_BAR) {
Width, Snp->PciIo->Mem.Write (
Snp->MemoryBarIndex, // BAR 0, Memory base address Snp->PciIo,
MemOrPortAddr, Width,
1, // count Snp->MemoryBarIndex, // BAR 0, Memory base address
(VOID *) (UINTN) BufferPtr MemOrPortAddr,
); 1, // count
(VOID *) (UINTN) BufferPtr
);
}
break; break;
} }

View File

@ -466,8 +466,8 @@ SimpleNetworkDriverStart (
// the IO BAR. Save the index of the BAR into the adapter info structure. // 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 // for regular 32bit BARs, 0 is memory mapped, 1 is io mapped
// //
Snp->MemoryBarIndex = 0; Snp->MemoryBarIndex = PCI_MAX_BAR;
Snp->IoBarIndex = 1; Snp->IoBarIndex = PCI_MAX_BAR;
FoundMemoryBar = FALSE; FoundMemoryBar = FALSE;
FoundIoBar = FALSE; FoundIoBar = FALSE;
for (BarIndex = 0; BarIndex < PCI_MAX_BAR; BarIndex++) { for (BarIndex = 0; BarIndex < PCI_MAX_BAR; BarIndex++) {