* [PATCH] ArmPkg/Library/ArmDmaLib: Deallocate Map buffer in case of error
@ 2016-09-22 22:37 Daniil Egranov
2016-09-23 8:00 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Daniil Egranov @ 2016-09-22 22:37 UTC (permalink / raw)
To: edk2-devel; +Cc: leif.lindholm, Daniil Egranov
The patch is fixing memory leak in case of errors.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
---
ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
index d48d6ff..e0006c0 100644
--- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
+++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
@@ -90,14 +90,13 @@ DmaMap (
return EFI_OUT_OF_RESOURCES;
}
- *Mapping = Map;
-
if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) ||
((*NumberOfBytes & (gCacheAlignment - 1)) != 0)) {
// Get the cacheability of the region
Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor);
if (EFI_ERROR(Status)) {
+ FreePool(Map);
return Status;
}
@@ -112,6 +111,7 @@ DmaMap (
"%a: Operation type 'MapOperationBusMasterCommonBuffer' is only supported\n"
"on memory regions that were allocated using DmaAllocateBuffer ()\n",
__FUNCTION__));
+ FreePool(Map);
return EFI_UNSUPPORTED;
}
@@ -122,6 +122,7 @@ DmaMap (
Map->DoubleBuffer = TRUE;
Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES (*NumberOfBytes), &Buffer);
if (EFI_ERROR (Status)) {
+ FreePool(Map);
return Status;
}
@@ -162,6 +163,8 @@ DmaMap (
Map->NumberOfBytes = *NumberOfBytes;
Map->Operation = Operation;
+ *Mapping = Map;
+
return EFI_SUCCESS;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Library/ArmDmaLib: Deallocate Map buffer in case of error
2016-09-22 22:37 [PATCH] ArmPkg/Library/ArmDmaLib: Deallocate Map buffer in case of error Daniil Egranov
@ 2016-09-23 8:00 ` Ard Biesheuvel
2016-11-01 17:58 ` Ryan Harkin
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2016-09-23 8:00 UTC (permalink / raw)
To: Daniil Egranov; +Cc: edk2-devel@lists.01.org, Leif Lindholm
On 22 September 2016 at 23:37, Daniil Egranov <daniil.egranov@arm.com> wrote:
> The patch is fixing memory leak in case of errors.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> index d48d6ff..e0006c0 100644
> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> @@ -90,14 +90,13 @@ DmaMap (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - *Mapping = Map;
> -
> if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) ||
> ((*NumberOfBytes & (gCacheAlignment - 1)) != 0)) {
>
> // Get the cacheability of the region
> Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor);
> if (EFI_ERROR(Status)) {
> + FreePool(Map);
> return Status;
> }
>
> @@ -112,6 +111,7 @@ DmaMap (
> "%a: Operation type 'MapOperationBusMasterCommonBuffer' is only supported\n"
> "on memory regions that were allocated using DmaAllocateBuffer ()\n",
> __FUNCTION__));
> + FreePool(Map);
> return EFI_UNSUPPORTED;
> }
>
> @@ -122,6 +122,7 @@ DmaMap (
> Map->DoubleBuffer = TRUE;
> Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES (*NumberOfBytes), &Buffer);
> if (EFI_ERROR (Status)) {
> + FreePool(Map);
> return Status;
> }
>
> @@ -162,6 +163,8 @@ DmaMap (
> Map->NumberOfBytes = *NumberOfBytes;
> Map->Operation = Operation;
>
> + *Mapping = Map;
> +
> return EFI_SUCCESS;
> }
>
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Library/ArmDmaLib: Deallocate Map buffer in case of error
2016-09-23 8:00 ` Ard Biesheuvel
@ 2016-11-01 17:58 ` Ryan Harkin
2016-11-02 11:56 ` Ryan Harkin
0 siblings, 1 reply; 5+ messages in thread
From: Ryan Harkin @ 2016-11-01 17:58 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Daniil Egranov, edk2-devel@lists.01.org, Leif Lindholm
Hi Leif,
Is this waiting for a Tested-by or something else?
Cheers,
Ryan.
On 23 September 2016 at 09:00, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 22 September 2016 at 23:37, Daniil Egranov <daniil.egranov@arm.com> wrote:
>> The patch is fixing memory leak in case of errors.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
>> ---
>> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>> index d48d6ff..e0006c0 100644
>> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>> @@ -90,14 +90,13 @@ DmaMap (
>> return EFI_OUT_OF_RESOURCES;
>> }
>>
>> - *Mapping = Map;
>> -
>> if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) ||
>> ((*NumberOfBytes & (gCacheAlignment - 1)) != 0)) {
>>
>> // Get the cacheability of the region
>> Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor);
>> if (EFI_ERROR(Status)) {
>> + FreePool(Map);
>> return Status;
>> }
>>
>> @@ -112,6 +111,7 @@ DmaMap (
>> "%a: Operation type 'MapOperationBusMasterCommonBuffer' is only supported\n"
>> "on memory regions that were allocated using DmaAllocateBuffer ()\n",
>> __FUNCTION__));
>> + FreePool(Map);
>> return EFI_UNSUPPORTED;
>> }
>>
>> @@ -122,6 +122,7 @@ DmaMap (
>> Map->DoubleBuffer = TRUE;
>> Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES (*NumberOfBytes), &Buffer);
>> if (EFI_ERROR (Status)) {
>> + FreePool(Map);
>> return Status;
>> }
>>
>> @@ -162,6 +163,8 @@ DmaMap (
>> Map->NumberOfBytes = *NumberOfBytes;
>> Map->Operation = Operation;
>>
>> + *Mapping = Map;
>> +
>> return EFI_SUCCESS;
>> }
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Library/ArmDmaLib: Deallocate Map buffer in case of error
2016-11-01 17:58 ` Ryan Harkin
@ 2016-11-02 11:56 ` Ryan Harkin
2016-11-15 13:28 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Ryan Harkin @ 2016-11-02 11:56 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Daniil Egranov, edk2-devel@lists.01.org, Leif Lindholm
On 1 November 2016 at 17:58, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> Hi Leif,
>
> Is this waiting for a Tested-by or something else?
>
Either way, I tested it on Juno R0/1/2 while I was testing the MAC
address change...
> Cheers,
> Ryan.
>
> On 23 September 2016 at 09:00, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 22 September 2016 at 23:37, Daniil Egranov <daniil.egranov@arm.com> wrote:
>>> The patch is fixing memory leak in case of errors.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
Tested-by; Ryan Harkin <ryan.harkin@linaro.org>
>>> ---
>>> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>>> index d48d6ff..e0006c0 100644
>>> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>>> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>>> @@ -90,14 +90,13 @@ DmaMap (
>>> return EFI_OUT_OF_RESOURCES;
>>> }
>>>
>>> - *Mapping = Map;
>>> -
>>> if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) ||
>>> ((*NumberOfBytes & (gCacheAlignment - 1)) != 0)) {
>>>
>>> // Get the cacheability of the region
>>> Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor);
>>> if (EFI_ERROR(Status)) {
>>> + FreePool(Map);
>>> return Status;
>>> }
>>>
>>> @@ -112,6 +111,7 @@ DmaMap (
>>> "%a: Operation type 'MapOperationBusMasterCommonBuffer' is only supported\n"
>>> "on memory regions that were allocated using DmaAllocateBuffer ()\n",
>>> __FUNCTION__));
>>> + FreePool(Map);
>>> return EFI_UNSUPPORTED;
>>> }
>>>
>>> @@ -122,6 +122,7 @@ DmaMap (
>>> Map->DoubleBuffer = TRUE;
>>> Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES (*NumberOfBytes), &Buffer);
>>> if (EFI_ERROR (Status)) {
>>> + FreePool(Map);
>>> return Status;
>>> }
>>>
>>> @@ -162,6 +163,8 @@ DmaMap (
>>> Map->NumberOfBytes = *NumberOfBytes;
>>> Map->Operation = Operation;
>>>
>>> + *Mapping = Map;
>>> +
>>> return EFI_SUCCESS;
>>> }
>>>
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/Library/ArmDmaLib: Deallocate Map buffer in case of error
2016-11-02 11:56 ` Ryan Harkin
@ 2016-11-15 13:28 ` Ard Biesheuvel
0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2016-11-15 13:28 UTC (permalink / raw)
To: Ryan Harkin; +Cc: Daniil Egranov, edk2-devel@lists.01.org, Leif Lindholm
On 2 November 2016 at 11:56, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> On 1 November 2016 at 17:58, Ryan Harkin <ryan.harkin@linaro.org> wrote:
>> Hi Leif,
>>
>> Is this waiting for a Tested-by or something else?
>>
>
> Either way, I tested it on Juno R0/1/2 while I was testing the MAC
> address change...
>
>
>> Cheers,
>> Ryan.
>>
>> On 23 September 2016 at 09:00, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> On 22 September 2016 at 23:37, Daniil Egranov <daniil.egranov@arm.com> wrote:
>>>> The patch is fixing memory leak in case of errors.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
>>>
>>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>
>
> Tested-by; Ryan Harkin <ryan.harkin@linaro.org>
>
Pushed, thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-15 13:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-22 22:37 [PATCH] ArmPkg/Library/ArmDmaLib: Deallocate Map buffer in case of error Daniil Egranov
2016-09-23 8:00 ` Ard Biesheuvel
2016-11-01 17:58 ` Ryan Harkin
2016-11-02 11:56 ` Ryan Harkin
2016-11-15 13:28 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox