diff mbox series

[edk2] MdeModulePkg/SdMmcPciHcDxe: use FreeBuffer not FreePages from DMA buffer

Message ID 20171104214012.30888-1-ard.biesheuvel@linaro.org
State New
Headers show
Series [edk2] MdeModulePkg/SdMmcPciHcDxe: use FreeBuffer not FreePages from DMA buffer | expand

Commit Message

Ard Biesheuvel Nov. 4, 2017, 9:40 p.m. UTC
Don't use EFI_PCI_IO_PROTOCOL::FreePages () to free an allocation
created with EFI_PCI_IO_PROTOCOL::AllocatePages (). It is simply
incorrect, but given that it may interfere with IOMMU DMA protection
and/or memory encryption, it could pose a security risk as well.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Ard Biesheuvel Nov. 4, 2017, 10:10 p.m. UTC | #1
On 4 November 2017 at 21:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Don't use EFI_PCI_IO_PROTOCOL::FreePages () to free an allocation

> created with EFI_PCI_IO_PROTOCOL::AllocatePages (). It is simply

> incorrect, but given that it may interfere with IOMMU DMA protection

> and/or memory encryption, it could pose a security risk as well.

>

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c

> index 23faec5e2be0..a6005dd33bce 100644

> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c

> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c

> @@ -1009,7 +1009,7 @@ SdMmcPassThruPassThru (

>

>  Done:

>    if ((Trb != NULL) && (Trb->AdmaDesc != NULL)) {

> -    FreePages (Trb->AdmaDesc, Trb->AdmaPages);

> +    Private->PciIo->FreeBuffer (Private->PciIo, Trb->AdmaPages, Trb->AdmaDesc);


Actually, looking a bit deeper, it seems we need to simply call
SdMmcFreeTrb () here, otherwise PciIo->Unmap() is never called on the
DataMap mapping, which breaks non-coherent DMA (and also coherent
bounce buffering DMA)


>    }

>

>    if (Trb != NULL) {

> --

> 2.11.0

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index 23faec5e2be0..a6005dd33bce 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -1009,7 +1009,7 @@  SdMmcPassThruPassThru (
 
 Done:
   if ((Trb != NULL) && (Trb->AdmaDesc != NULL)) {
-    FreePages (Trb->AdmaDesc, Trb->AdmaPages);
+    Private->PciIo->FreeBuffer (Private->PciIo, Trb->AdmaPages, Trb->AdmaDesc);
   }
 
   if (Trb != NULL) {