* [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer
@ 2018-09-06 18:55 Vladimir Olovyannikov
2018-09-06 18:55 ` [PATCH 2/3] EmbeddedPkg/CoherentDmaLib: Add missing checks to DmaMap Vladimir Olovyannikov
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Vladimir Olovyannikov @ 2018-09-06 18:55 UTC (permalink / raw)
To: edk2-devel, Ard Biesheuvel, Leif Lindholm, Ruiyu Ni; +Cc: Vladimir Olovyannikov
The only valid memory types for DmaAlignedBuffer should be
EfiBootServicesData and EfiRuntimeServicesData. However due to the typo,
there is no way to allocate runtime pages, and INVALID_PARAMETER is
always returned. Fix the typo.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
index 564db83c901c..8ca9e6aa5b1b 100644
--- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
@@ -154,7 +154,7 @@ DmaAllocateAlignedBuffer (
//
if (MemoryType == EfiBootServicesData) {
*HostAddress = AllocateAlignedPages (Pages, Alignment);
- } else if (MemoryType != EfiRuntimeServicesData) {
+ } else if (MemoryType == EfiRuntimeServicesData) {
*HostAddress = AllocateAlignedRuntimePages (Pages, Alignment);
} else {
return EFI_INVALID_PARAMETER;
--
2.18.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] EmbeddedPkg/CoherentDmaLib: Add missing checks to DmaMap
2018-09-06 18:55 [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer Vladimir Olovyannikov
@ 2018-09-06 18:55 ` Vladimir Olovyannikov
2018-09-06 18:55 ` [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation Vladimir Olovyannikov
2018-09-07 10:37 ` [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer Ard Biesheuvel
2 siblings, 0 replies; 8+ messages in thread
From: Vladimir Olovyannikov @ 2018-09-06 18:55 UTC (permalink / raw)
To: edk2-devel, Ard Biesheuvel, Leif Lindholm, Ruiyu Ni; +Cc: Vladimir Olovyannikov
UEFI Sct validates Dma mapping. For CoherentDmaLib it always failed
because there were no required checks present in DmaMap.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
index 8ca9e6aa5b1b..eb88fa288a99 100644
--- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
@@ -58,6 +58,12 @@ DmaMap (
OUT VOID **Mapping
)
{
+ if (HostAddress == NULL ||
+ NumberOfBytes == NULL ||
+ DeviceAddress == NULL ||
+ Mapping == NULL ) {
+ return EFI_INVALID_PARAMETER;
+ }
*DeviceAddress = HostToDeviceAddress (HostAddress);
*Mapping = NULL;
return EFI_SUCCESS;
--
2.18.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation
2018-09-06 18:55 [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer Vladimir Olovyannikov
2018-09-06 18:55 ` [PATCH 2/3] EmbeddedPkg/CoherentDmaLib: Add missing checks to DmaMap Vladimir Olovyannikov
@ 2018-09-06 18:55 ` Vladimir Olovyannikov
2018-09-07 10:36 ` Ard Biesheuvel
2018-09-07 10:37 ` [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer Ard Biesheuvel
2 siblings, 1 reply; 8+ messages in thread
From: Vladimir Olovyannikov @ 2018-09-06 18:55 UTC (permalink / raw)
To: edk2-devel, Ard Biesheuvel, Leif Lindholm, Ruiyu Ni; +Cc: Vladimir Olovyannikov
UEFI SCT crashed and failed in NonDiscoverablePciDeviceDxe becase
required checks were not performed. Perform parameters validation in
NonDiscoverablePciDeviceDxe.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
.../NonDiscoverablePciDeviceIo.c | 50 ++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 0e42ae4bf6ec..07118d59fd68 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -52,6 +52,10 @@ GetBarResource (
BarIndex -= (UINT8)Dev->BarOffset;
+ if (BarIndex >= Dev->BarCount) {
+ return EFI_UNSUPPORTED;
+ }
+
for (Desc = Dev->Device->Resources;
Desc->Desc != ACPI_END_TAG_DESCRIPTOR;
Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) {
@@ -597,6 +601,19 @@ CoherentPciIoMap (
EFI_STATUS Status;
NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo;
+ if (Operation != EfiPciIoOperationBusMasterRead &&
+ Operation != EfiPciIoOperationBusMasterWrite &&
+ Operation != EfiPciIoOperationBusMasterCommonBuffer) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (HostAddress == NULL ||
+ NumberOfBytes == NULL ||
+ DeviceAddress == NULL ||
+ Mapping == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
//
// If HostAddress exceeds 4 GB, and this device does not support 64-bit DMA
// addressing, we need to allocate a bounce buffer and copy over the data.
@@ -720,6 +737,11 @@ CoherentPciIoAllocateBuffer (
return EFI_UNSUPPORTED;
}
+ if ((MemoryType != EfiBootServicesData) &&
+ (MemoryType != EfiRuntimeServicesData)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
//
// Allocate below 4 GB if the dual address cycle attribute has not
// been set. If the system has no memory available below 4 GB, there
@@ -877,6 +899,10 @@ NonCoherentPciIoAllocateBuffer (
NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
VOID *AllocAddress;
+ if (HostAddress == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
Status = CoherentPciIoAllocateBuffer (This, Type, MemoryType, Pages,
@@ -995,6 +1021,19 @@ NonCoherentPciIoMap (
EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
BOOLEAN Bounce;
+ if (HostAddress == NULL ||
+ NumberOfBytes == NULL ||
+ DeviceAddress == NULL ||
+ Mapping == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Operation != EfiPciIoOperationBusMasterRead &&
+ Operation != EfiPciIoOperationBusMasterWrite &&
+ Operation != EfiPciIoOperationBusMasterCommonBuffer) {
+ return EFI_INVALID_PARAMETER;
+ }
+
MapInfo = AllocatePool (sizeof *MapInfo);
if (MapInfo == NULL) {
return EFI_OUT_OF_RESOURCES;
@@ -1228,8 +1267,17 @@ PciIoAttributes (
NON_DISCOVERABLE_PCI_DEVICE *Dev;
BOOLEAN Enable;
+ #define DEV_SUPPORTED_ATTRIBUTES \
+ (EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
+
Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+ if (Attributes) {
+ if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
+ return EFI_UNSUPPORTED;
+ }
+ }
+
Enable = FALSE;
switch (Operation) {
case EfiPciIoAttributeOperationGet:
@@ -1243,7 +1291,7 @@ PciIoAttributes (
if (Result == NULL) {
return EFI_INVALID_PARAMETER;
}
- *Result = EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
+ *Result = DEV_SUPPORTED_ATTRIBUTES;
break;
case EfiPciIoAttributeOperationEnable:
--
2.18.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation
2018-09-06 18:55 ` [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation Vladimir Olovyannikov
@ 2018-09-07 10:36 ` Ard Biesheuvel
2018-12-15 13:36 ` Leif Lindholm
0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2018-09-07 10:36 UTC (permalink / raw)
To: Vladimir Olovyannikov, Zeng, Star
Cc: edk2-devel@lists.01.org, Leif Lindholm, Ruiyu Ni
On 6 September 2018 at 20:55, Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com> wrote:
> UEFI SCT crashed and failed in NonDiscoverablePciDeviceDxe becase
> required checks were not performed. Perform parameters validation in
> NonDiscoverablePciDeviceDxe.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> .../NonDiscoverablePciDeviceIo.c | 50 ++++++++++++++++++-
> 1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> index 0e42ae4bf6ec..07118d59fd68 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> @@ -52,6 +52,10 @@ GetBarResource (
>
> BarIndex -= (UINT8)Dev->BarOffset;
>
> + if (BarIndex >= Dev->BarCount) {
> + return EFI_UNSUPPORTED;
> + }
> +
> for (Desc = Dev->Device->Resources;
> Desc->Desc != ACPI_END_TAG_DESCRIPTOR;
> Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) {
> @@ -597,6 +601,19 @@ CoherentPciIoMap (
> EFI_STATUS Status;
> NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo;
>
> + if (Operation != EfiPciIoOperationBusMasterRead &&
> + Operation != EfiPciIoOperationBusMasterWrite &&
> + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (HostAddress == NULL ||
> + NumberOfBytes == NULL ||
> + DeviceAddress == NULL ||
> + Mapping == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> //
> // If HostAddress exceeds 4 GB, and this device does not support 64-bit DMA
> // addressing, we need to allocate a bounce buffer and copy over the data.
> @@ -720,6 +737,11 @@ CoherentPciIoAllocateBuffer (
> return EFI_UNSUPPORTED;
> }
>
> + if ((MemoryType != EfiBootServicesData) &&
> + (MemoryType != EfiRuntimeServicesData)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> //
> // Allocate below 4 GB if the dual address cycle attribute has not
> // been set. If the system has no memory available below 4 GB, there
> @@ -877,6 +899,10 @@ NonCoherentPciIoAllocateBuffer (
> NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
> VOID *AllocAddress;
>
> + if (HostAddress == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
>
> Status = CoherentPciIoAllocateBuffer (This, Type, MemoryType, Pages,
> @@ -995,6 +1021,19 @@ NonCoherentPciIoMap (
> EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
> BOOLEAN Bounce;
>
> + if (HostAddress == NULL ||
> + NumberOfBytes == NULL ||
> + DeviceAddress == NULL ||
> + Mapping == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (Operation != EfiPciIoOperationBusMasterRead &&
> + Operation != EfiPciIoOperationBusMasterWrite &&
> + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> MapInfo = AllocatePool (sizeof *MapInfo);
> if (MapInfo == NULL) {
> return EFI_OUT_OF_RESOURCES;
> @@ -1228,8 +1267,17 @@ PciIoAttributes (
> NON_DISCOVERABLE_PCI_DEVICE *Dev;
> BOOLEAN Enable;
>
> + #define DEV_SUPPORTED_ATTRIBUTES \
> + (EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> +
> Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
>
> + if (Attributes) {
> + if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
> + return EFI_UNSUPPORTED;
> + }
> + }
> +
> Enable = FALSE;
> switch (Operation) {
> case EfiPciIoAttributeOperationGet:
> @@ -1243,7 +1291,7 @@ PciIoAttributes (
> if (Result == NULL) {
> return EFI_INVALID_PARAMETER;
> }
> - *Result = EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> + *Result = DEV_SUPPORTED_ATTRIBUTES;
> break;
>
> case EfiPciIoAttributeOperationEnable:
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer
2018-09-06 18:55 [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer Vladimir Olovyannikov
2018-09-06 18:55 ` [PATCH 2/3] EmbeddedPkg/CoherentDmaLib: Add missing checks to DmaMap Vladimir Olovyannikov
2018-09-06 18:55 ` [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation Vladimir Olovyannikov
@ 2018-09-07 10:37 ` Ard Biesheuvel
2 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2018-09-07 10:37 UTC (permalink / raw)
To: Vladimir Olovyannikov; +Cc: edk2-devel@lists.01.org, Leif Lindholm, Ruiyu Ni
On 6 September 2018 at 20:55, Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com> wrote:
> The only valid memory types for DmaAlignedBuffer should be
> EfiBootServicesData and EfiRuntimeServicesData. However due to the typo,
> there is no way to allocate runtime pages, and INVALID_PARAMETER is
> always returned. Fix the typo.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Patches #1 and #2
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Pushed as 4d621893471c..c4709260f62f
Thanks!
> ---
> EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> index 564db83c901c..8ca9e6aa5b1b 100644
> --- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> +++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> @@ -154,7 +154,7 @@ DmaAllocateAlignedBuffer (
> //
> if (MemoryType == EfiBootServicesData) {
> *HostAddress = AllocateAlignedPages (Pages, Alignment);
> - } else if (MemoryType != EfiRuntimeServicesData) {
> + } else if (MemoryType == EfiRuntimeServicesData) {
> *HostAddress = AllocateAlignedRuntimePages (Pages, Alignment);
> } else {
> return EFI_INVALID_PARAMETER;
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation
2018-09-07 10:36 ` Ard Biesheuvel
@ 2018-12-15 13:36 ` Leif Lindholm
2018-12-17 0:16 ` Wang, Jian J
2018-12-17 0:59 ` Wang, Jian J
0 siblings, 2 replies; 8+ messages in thread
From: Leif Lindholm @ 2018-12-15 13:36 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Vladimir Olovyannikov, Zeng, Star, edk2-devel@lists.01.org,
Ruiyu Ni, Jian J Wang, Hao Wu
Jian, Hao,
I guess Ray and Star are no longer maintainers of this package.
This patch never got pushed - could you have a look please?
Regards,
Leif
On Fri, Sep 07, 2018 at 12:36:32PM +0200, Ard Biesheuvel wrote:
> On 6 September 2018 at 20:55, Vladimir Olovyannikov
> <vladimir.olovyannikov@broadcom.com> wrote:
> > UEFI SCT crashed and failed in NonDiscoverablePciDeviceDxe becase
> > required checks were not performed. Perform parameters validation in
> > NonDiscoverablePciDeviceDxe.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> > ---
> > .../NonDiscoverablePciDeviceIo.c | 50 ++++++++++++++++++-
> > 1 file changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> > index 0e42ae4bf6ec..07118d59fd68 100644
> > --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> > +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> > @@ -52,6 +52,10 @@ GetBarResource (
> >
> > BarIndex -= (UINT8)Dev->BarOffset;
> >
> > + if (BarIndex >= Dev->BarCount) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > for (Desc = Dev->Device->Resources;
> > Desc->Desc != ACPI_END_TAG_DESCRIPTOR;
> > Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) {
> > @@ -597,6 +601,19 @@ CoherentPciIoMap (
> > EFI_STATUS Status;
> > NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo;
> >
> > + if (Operation != EfiPciIoOperationBusMasterRead &&
> > + Operation != EfiPciIoOperationBusMasterWrite &&
> > + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (HostAddress == NULL ||
> > + NumberOfBytes == NULL ||
> > + DeviceAddress == NULL ||
> > + Mapping == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > //
> > // If HostAddress exceeds 4 GB, and this device does not support 64-bit DMA
> > // addressing, we need to allocate a bounce buffer and copy over the data.
> > @@ -720,6 +737,11 @@ CoherentPciIoAllocateBuffer (
> > return EFI_UNSUPPORTED;
> > }
> >
> > + if ((MemoryType != EfiBootServicesData) &&
> > + (MemoryType != EfiRuntimeServicesData)) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > //
> > // Allocate below 4 GB if the dual address cycle attribute has not
> > // been set. If the system has no memory available below 4 GB, there
> > @@ -877,6 +899,10 @@ NonCoherentPciIoAllocateBuffer (
> > NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
> > VOID *AllocAddress;
> >
> > + if (HostAddress == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> >
> > Status = CoherentPciIoAllocateBuffer (This, Type, MemoryType, Pages,
> > @@ -995,6 +1021,19 @@ NonCoherentPciIoMap (
> > EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
> > BOOLEAN Bounce;
> >
> > + if (HostAddress == NULL ||
> > + NumberOfBytes == NULL ||
> > + DeviceAddress == NULL ||
> > + Mapping == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + if (Operation != EfiPciIoOperationBusMasterRead &&
> > + Operation != EfiPciIoOperationBusMasterWrite &&
> > + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > MapInfo = AllocatePool (sizeof *MapInfo);
> > if (MapInfo == NULL) {
> > return EFI_OUT_OF_RESOURCES;
> > @@ -1228,8 +1267,17 @@ PciIoAttributes (
> > NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > BOOLEAN Enable;
> >
> > + #define DEV_SUPPORTED_ATTRIBUTES \
> > + (EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> > +
> > Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> >
> > + if (Attributes) {
> > + if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
> > + return EFI_UNSUPPORTED;
> > + }
> > + }
> > +
> > Enable = FALSE;
> > switch (Operation) {
> > case EfiPciIoAttributeOperationGet:
> > @@ -1243,7 +1291,7 @@ PciIoAttributes (
> > if (Result == NULL) {
> > return EFI_INVALID_PARAMETER;
> > }
> > - *Result = EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> > + *Result = DEV_SUPPORTED_ATTRIBUTES;
> > break;
> >
> > case EfiPciIoAttributeOperationEnable:
> > --
> > 2.18.0
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation
2018-12-15 13:36 ` Leif Lindholm
@ 2018-12-17 0:16 ` Wang, Jian J
2018-12-17 0:59 ` Wang, Jian J
1 sibling, 0 replies; 8+ messages in thread
From: Wang, Jian J @ 2018-12-17 0:16 UTC (permalink / raw)
To: Leif Lindholm, Ard Biesheuvel
Cc: Vladimir Olovyannikov, Zeng, Star, edk2-devel@lists.01.org,
Ni, Ruiyu, Wu, Hao A
Leif,
Thanks for reminding. I'll take care of it.
Regards,
Jian
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Saturday, December 15, 2018 9:37 PM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; Zeng, Star
> <star.zeng@intel.com>; edk2-devel@lists.01.org; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>
> Subject: Re: [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add
> missing validation
>
> Jian, Hao,
>
> I guess Ray and Star are no longer maintainers of this package.
> This patch never got pushed - could you have a look please?
>
> Regards,
>
> Leif
>
> On Fri, Sep 07, 2018 at 12:36:32PM +0200, Ard Biesheuvel wrote:
> > On 6 September 2018 at 20:55, Vladimir Olovyannikov
> > <vladimir.olovyannikov@broadcom.com> wrote:
> > > UEFI SCT crashed and failed in NonDiscoverablePciDeviceDxe becase
> > > required checks were not performed. Perform parameters validation in
> > > NonDiscoverablePciDeviceDxe.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Vladimir Olovyannikov
> <vladimir.olovyannikov@broadcom.com>
> >
> > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > > ---
> > > .../NonDiscoverablePciDeviceIo.c | 50 ++++++++++++++++++-
> > > 1 file changed, 49 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> > > index 0e42ae4bf6ec..07118d59fd68 100644
> > > ---
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> > > +++
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> > > @@ -52,6 +52,10 @@ GetBarResource (
> > >
> > > BarIndex -= (UINT8)Dev->BarOffset;
> > >
> > > + if (BarIndex >= Dev->BarCount) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > +
> > > for (Desc = Dev->Device->Resources;
> > > Desc->Desc != ACPI_END_TAG_DESCRIPTOR;
> > > Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) {
> > > @@ -597,6 +601,19 @@ CoherentPciIoMap (
> > > EFI_STATUS Status;
> > > NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo;
> > >
> > > + if (Operation != EfiPciIoOperationBusMasterRead &&
> > > + Operation != EfiPciIoOperationBusMasterWrite &&
> > > + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > + if (HostAddress == NULL ||
> > > + NumberOfBytes == NULL ||
> > > + DeviceAddress == NULL ||
> > > + Mapping == NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > //
> > > // If HostAddress exceeds 4 GB, and this device does not support 64-bit
> DMA
> > > // addressing, we need to allocate a bounce buffer and copy over the data.
> > > @@ -720,6 +737,11 @@ CoherentPciIoAllocateBuffer (
> > > return EFI_UNSUPPORTED;
> > > }
> > >
> > > + if ((MemoryType != EfiBootServicesData) &&
> > > + (MemoryType != EfiRuntimeServicesData)) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > //
> > > // Allocate below 4 GB if the dual address cycle attribute has not
> > > // been set. If the system has no memory available below 4 GB, there
> > > @@ -877,6 +899,10 @@ NonCoherentPciIoAllocateBuffer (
> > > NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
> > > VOID *AllocAddress;
> > >
> > > + if (HostAddress == NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > >
> > > Status = CoherentPciIoAllocateBuffer (This, Type, MemoryType, Pages,
> > > @@ -995,6 +1021,19 @@ NonCoherentPciIoMap (
> > > EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
> > > BOOLEAN Bounce;
> > >
> > > + if (HostAddress == NULL ||
> > > + NumberOfBytes == NULL ||
> > > + DeviceAddress == NULL ||
> > > + Mapping == NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > + if (Operation != EfiPciIoOperationBusMasterRead &&
> > > + Operation != EfiPciIoOperationBusMasterWrite &&
> > > + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > MapInfo = AllocatePool (sizeof *MapInfo);
> > > if (MapInfo == NULL) {
> > > return EFI_OUT_OF_RESOURCES;
> > > @@ -1228,8 +1267,17 @@ PciIoAttributes (
> > > NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > > BOOLEAN Enable;
> > >
> > > + #define DEV_SUPPORTED_ATTRIBUTES \
> > > + (EFI_PCI_DEVICE_ENABLE |
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> > > +
> > > Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > >
> > > + if (Attributes) {
> > > + if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > + }
> > > +
> > > Enable = FALSE;
> > > switch (Operation) {
> > > case EfiPciIoAttributeOperationGet:
> > > @@ -1243,7 +1291,7 @@ PciIoAttributes (
> > > if (Result == NULL) {
> > > return EFI_INVALID_PARAMETER;
> > > }
> > > - *Result = EFI_PCI_DEVICE_ENABLE |
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> > > + *Result = DEV_SUPPORTED_ATTRIBUTES;
> > > break;
> > >
> > > case EfiPciIoAttributeOperationEnable:
> > > --
> > > 2.18.0
> > >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation
2018-12-15 13:36 ` Leif Lindholm
2018-12-17 0:16 ` Wang, Jian J
@ 2018-12-17 0:59 ` Wang, Jian J
1 sibling, 0 replies; 8+ messages in thread
From: Wang, Jian J @ 2018-12-17 0:59 UTC (permalink / raw)
To: Leif Lindholm, Ard Biesheuvel
Cc: Vladimir Olovyannikov, Zeng, Star, edk2-devel@lists.01.org,
Ni, Ruiyu, Wu, Hao A
Push @ c8c3c53669bea887ecc093167d64d1fbe63c213f
Regards,
Jian
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Saturday, December 15, 2018 9:37 PM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; Zeng, Star
> <star.zeng@intel.com>; edk2-devel@lists.01.org; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>
> Subject: Re: [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add
> missing validation
>
> Jian, Hao,
>
> I guess Ray and Star are no longer maintainers of this package.
> This patch never got pushed - could you have a look please?
>
> Regards,
>
> Leif
>
> On Fri, Sep 07, 2018 at 12:36:32PM +0200, Ard Biesheuvel wrote:
> > On 6 September 2018 at 20:55, Vladimir Olovyannikov
> > <vladimir.olovyannikov@broadcom.com> wrote:
> > > UEFI SCT crashed and failed in NonDiscoverablePciDeviceDxe becase
> > > required checks were not performed. Perform parameters validation in
> > > NonDiscoverablePciDeviceDxe.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Vladimir Olovyannikov
> <vladimir.olovyannikov@broadcom.com>
> >
> > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > > ---
> > > .../NonDiscoverablePciDeviceIo.c | 50 ++++++++++++++++++-
> > > 1 file changed, 49 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> > > index 0e42ae4bf6ec..07118d59fd68 100644
> > > ---
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> > > +++
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> > > @@ -52,6 +52,10 @@ GetBarResource (
> > >
> > > BarIndex -= (UINT8)Dev->BarOffset;
> > >
> > > + if (BarIndex >= Dev->BarCount) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > +
> > > for (Desc = Dev->Device->Resources;
> > > Desc->Desc != ACPI_END_TAG_DESCRIPTOR;
> > > Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) {
> > > @@ -597,6 +601,19 @@ CoherentPciIoMap (
> > > EFI_STATUS Status;
> > > NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo;
> > >
> > > + if (Operation != EfiPciIoOperationBusMasterRead &&
> > > + Operation != EfiPciIoOperationBusMasterWrite &&
> > > + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > + if (HostAddress == NULL ||
> > > + NumberOfBytes == NULL ||
> > > + DeviceAddress == NULL ||
> > > + Mapping == NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > //
> > > // If HostAddress exceeds 4 GB, and this device does not support 64-bit
> DMA
> > > // addressing, we need to allocate a bounce buffer and copy over the data.
> > > @@ -720,6 +737,11 @@ CoherentPciIoAllocateBuffer (
> > > return EFI_UNSUPPORTED;
> > > }
> > >
> > > + if ((MemoryType != EfiBootServicesData) &&
> > > + (MemoryType != EfiRuntimeServicesData)) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > //
> > > // Allocate below 4 GB if the dual address cycle attribute has not
> > > // been set. If the system has no memory available below 4 GB, there
> > > @@ -877,6 +899,10 @@ NonCoherentPciIoAllocateBuffer (
> > > NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
> > > VOID *AllocAddress;
> > >
> > > + if (HostAddress == NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > >
> > > Status = CoherentPciIoAllocateBuffer (This, Type, MemoryType, Pages,
> > > @@ -995,6 +1021,19 @@ NonCoherentPciIoMap (
> > > EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
> > > BOOLEAN Bounce;
> > >
> > > + if (HostAddress == NULL ||
> > > + NumberOfBytes == NULL ||
> > > + DeviceAddress == NULL ||
> > > + Mapping == NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > + if (Operation != EfiPciIoOperationBusMasterRead &&
> > > + Operation != EfiPciIoOperationBusMasterWrite &&
> > > + Operation != EfiPciIoOperationBusMasterCommonBuffer) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > MapInfo = AllocatePool (sizeof *MapInfo);
> > > if (MapInfo == NULL) {
> > > return EFI_OUT_OF_RESOURCES;
> > > @@ -1228,8 +1267,17 @@ PciIoAttributes (
> > > NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > > BOOLEAN Enable;
> > >
> > > + #define DEV_SUPPORTED_ATTRIBUTES \
> > > + (EFI_PCI_DEVICE_ENABLE |
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
> > > +
> > > Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > >
> > > + if (Attributes) {
> > > + if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > + }
> > > +
> > > Enable = FALSE;
> > > switch (Operation) {
> > > case EfiPciIoAttributeOperationGet:
> > > @@ -1243,7 +1291,7 @@ PciIoAttributes (
> > > if (Result == NULL) {
> > > return EFI_INVALID_PARAMETER;
> > > }
> > > - *Result = EFI_PCI_DEVICE_ENABLE |
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> > > + *Result = DEV_SUPPORTED_ATTRIBUTES;
> > > break;
> > >
> > > case EfiPciIoAttributeOperationEnable:
> > > --
> > > 2.18.0
> > >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-12-17 1:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-06 18:55 [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer Vladimir Olovyannikov
2018-09-06 18:55 ` [PATCH 2/3] EmbeddedPkg/CoherentDmaLib: Add missing checks to DmaMap Vladimir Olovyannikov
2018-09-06 18:55 ` [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation Vladimir Olovyannikov
2018-09-07 10:36 ` Ard Biesheuvel
2018-12-15 13:36 ` Leif Lindholm
2018-12-17 0:16 ` Wang, Jian J
2018-12-17 0:59 ` Wang, Jian J
2018-09-07 10:37 ` [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox