[PATCH] EmbeddedPkg/NonCoherentDmaLib: avoid dereferencing bogus buffer address

[PATCH] EmbeddedPkg/NonCoherentDmaLib: avoid dereferencing bogus buffer address

[Ard Biesheuvel]

The bounce buffering code in NonCoherentDmaLib copies data into the
bounce buffer using CopyMem(), but passes Map->HostAddress as the
source of the copy before it has been assigned its correct value.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
index 115345765435..9c8ef5bfb533 100644
--- a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
+++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
@@ -225,8 +225,7 @@ DmaMap (
     }
 
     if (Map->Operation == MapOperationBusMasterRead) {
-      CopyMem (Map->BufferAddress, (VOID *)(UINTN)Map->HostAddress,
-        *NumberOfBytes);
+      CopyMem (Map->BufferAddress, (VOID *)(UINTN)HostAddress, *NumberOfBytes);
     }
     mCpu->FlushDataCache (mCpu, (UINTN)Map->BufferAddress, AllocSize,
             EfiCpuFlushTypeWriteBack);
-- 
2.27.0

Re: [PATCH] EmbeddedPkg/NonCoherentDmaLib: avoid dereferencing bogus buffer address

[Leif Lindholm]

On Wed, Jun 17, 2020 at 17:38:24 +0200, Ard Biesheuvel wrote:
> The bounce buffering code in NonCoherentDmaLib copies data into the
> bounce buffer using CopyMem(), but passes Map->HostAddress as the
> source of the copy before it has been assigned its correct value.

Whoops.

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Reviewed-by: Leif Lindholm <leif@nuviainc.com>

> ---
>  EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
> index 115345765435..9c8ef5bfb533 100644
> --- a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
> +++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
> @@ -225,8 +225,7 @@ DmaMap (
>      }
>  
>      if (Map->Operation == MapOperationBusMasterRead) {
> -      CopyMem (Map->BufferAddress, (VOID *)(UINTN)Map->HostAddress,
> -        *NumberOfBytes);
> +      CopyMem (Map->BufferAddress, (VOID *)(UINTN)HostAddress, *NumberOfBytes);
>      }
>      mCpu->FlushDataCache (mCpu, (UINTN)Map->BufferAddress, AllocSize,
>              EfiCpuFlushTypeWriteBack);
> -- 
> 2.27.0
> 

Re: [edk2-devel] [PATCH] EmbeddedPkg/NonCoherentDmaLib: avoid dereferencing bogus buffer address

[Andrei Warkentin]

[-- Attachment #1: Type: text/plain, Size: 2976 bytes --]

Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ard Biesheuvel via groups.io <ard.biesheuvel=arm.com@groups.io>
Sent: Wednesday, June 17, 2020 10:38 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; Andrei Warkentin <awarkentin@vmware.com>; jeremy.linton@arm.com <jeremy.linton@arm.com>; Samer.El-Haj-Mahmoud@arm.com <Samer.El-Haj-Mahmoud@arm.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>
Subject: [edk2-devel] [PATCH] EmbeddedPkg/NonCoherentDmaLib: avoid dereferencing bogus buffer address

The bounce buffering code in NonCoherentDmaLib copies data into the
bounce buffer using CopyMem(), but passes Map->HostAddress as the
source of the copy before it has been assigned its correct value.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
index 115345765435..9c8ef5bfb533 100644
--- a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
+++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
@@ -225,8 +225,7 @@ DmaMap (
     }



     if (Map->Operation == MapOperationBusMasterRead) {

-      CopyMem (Map->BufferAddress, (VOID *)(UINTN)Map->HostAddress,

-        *NumberOfBytes);

+      CopyMem (Map->BufferAddress, (VOID *)(UINTN)HostAddress, *NumberOfBytes);

     }

     mCpu->FlushDataCache (mCpu, (UINTN)Map->BufferAddress, AllocSize,

             EfiCpuFlushTypeWriteBack);

--
2.27.0


-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61421): https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F61421&amp;data=02%7C01%7Cawarkentin%40vmware.com%7Ce0cab9a2b8a74880b69f08d812d48073%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637280051185406598&amp;sdata=NylhdsdCkUdiGZZB6ya0pCMs8vYFb8tpdjW29q1dJAQ%3D&amp;reserved=0
Mute This Topic: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F74939973%2F4387333&amp;data=02%7C01%7Cawarkentin%40vmware.com%7Ce0cab9a2b8a74880b69f08d812d48073%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637280051185406598&amp;sdata=2Wv1bPYhk2Ac32ItKenF9bmig5t%2FKJuTFVWZacFZvY8%3D&amp;reserved=0
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=02%7C01%7Cawarkentin%40vmware.com%7Ce0cab9a2b8a74880b69f08d812d48073%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637280051185406598&amp;sdata=4Yp5s5IOCF%2BY6OwqcDZmcmgn1crE9MxuL3TnHEGSMsM%3D&amp;reserved=0  [awarkentin@vmware.com]
-=-=-=-=-=-=


[-- Attachment #2: Type: text/html, Size: 5300 bytes --]

Re: [PATCH] EmbeddedPkg/NonCoherentDmaLib: avoid dereferencing bogus buffer address

[Ard Biesheuvel]

On 6/17/20 6:14 PM, Leif Lindholm wrote:
> On Wed, Jun 17, 2020 at 17:38:24 +0200, Ard Biesheuvel wrote:
>> The bounce buffering code in NonCoherentDmaLib copies data into the
>> bounce buffer using CopyMem(), but passes Map->HostAddress as the
>> source of the copy before it has been assigned its correct value.
> 
> Whoops.
> 
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> 
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> 

Thanks

Merged as 8f22a331b955bd3f8077c7fa83bafeec566d6718


>> ---
>>   EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
>> index 115345765435..9c8ef5bfb533 100644
>> --- a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
>> +++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
>> @@ -225,8 +225,7 @@ DmaMap (
>>       }
>>   
>>       if (Map->Operation == MapOperationBusMasterRead) {
>> -      CopyMem (Map->BufferAddress, (VOID *)(UINTN)Map->HostAddress,
>> -        *NumberOfBytes);
>> +      CopyMem (Map->BufferAddress, (VOID *)(UINTN)HostAddress, *NumberOfBytes);
>>       }
>>       mCpu->FlushDataCache (mCpu, (UINTN)Map->BufferAddress, AllocSize,
>>               EfiCpuFlushTypeWriteBack);
>> -- 
>> 2.27.0
>>