diff mbox series

[Linaro-uefi,Linaro-uefi,v1,01/21] Hisilicon/UpdateFdtDxe: fix memory overflow issue

Message ID 1490015485-53685-2-git-send-email-chenhui.sun@linaro.org
State Superseded
Headers show
Series D02/D03 platforms bug fix | expand

Commit Message

Chenhui Sun March 20, 2017, 1:11 p.m. UTC
The size of the updated DTB file may be increased, so we need to allocate
more memory than the original DTB size,or memory overflow may happen.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: shaochangliang <shaochangliang@huawei.com>
Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
---
 Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Leif Lindholm March 20, 2017, 3:49 p.m. UTC | #1
On Mon, Mar 20, 2017 at 09:11:05PM +0800, Chenhui Sun wrote:
> The size of the updated DTB file may be increased, so we need to allocate
> more memory than the original DTB size,or memory overflow may happen.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: shaochangliang <shaochangliang@huawei.com>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
> ---
>  Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
> index 8586e33..699a820 100644
> --- a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
> +++ b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
> @@ -112,7 +112,7 @@ EFIAPI UpdateFdt (
>      Size = (UINTN)fdt_totalsize ((VOID*)(PcdGet64(FdtFileAddress)));
>      NewFdtBlobSize = Size + ADD_FILE_LENGTH;
>  
> -    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(Size), &NewFdtBlobBase);
> +    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(NewFdtBlobSize), &NewFdtBlobBase);

This is clearly a fix.
However, can you clarify whether 
(VOID) CopyMem((VOID*)NewFdtBlobBase, Fdt, Size);
also need to be updated?

>      if (EFI_ERROR (Status))
>      {
>          return EFI_OUT_OF_RESOURCES;
> -- 
> 1.9.1
>
Chenhui Sun March 21, 2017, 8:05 a.m. UTC | #2
Hi Leif,


在 2017/3/20 23:49, Leif Lindholm 写道:
> On Mon, Mar 20, 2017 at 09:11:05PM +0800, Chenhui Sun wrote:
>> The size of the updated DTB file may be increased, so we need to allocate
>> more memory than the original DTB size,or memory overflow may happen.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: shaochangliang <shaochangliang@huawei.com>
>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
>> Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
>> ---
>>   Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
>> index 8586e33..699a820 100644
>> --- a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
>> +++ b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
>> @@ -112,7 +112,7 @@ EFIAPI UpdateFdt (
>>       Size = (UINTN)fdt_totalsize ((VOID*)(PcdGet64(FdtFileAddress)));
>>       NewFdtBlobSize = Size + ADD_FILE_LENGTH;
>>   
>> -    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(Size), &NewFdtBlobBase);
>> +    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(NewFdtBlobSize), &NewFdtBlobBase);
> This is clearly a fix.
> However, can you clarify whether
> (VOID) CopyMem((VOID*)NewFdtBlobBase, Fdt, Size);
> also need to be updated?
Copy the source fdt file to NewFdtBlobBase, then update it, the size 
may  increase after updating,
so there just copy the origin data.

thanks.
Chenhui
>
>>       if (EFI_ERROR (Status))
>>       {
>>           return EFI_OUT_OF_RESOURCES;
>> -- 
>> 1.9.1
>>
Leif Lindholm March 21, 2017, 11:22 a.m. UTC | #3
On Tue, Mar 21, 2017 at 04:05:35PM +0800, Chenhui Sun wrote:
> Hi Leif,
> 
> 
> 在 2017/3/20 23:49, Leif Lindholm 写道:
> >On Mon, Mar 20, 2017 at 09:11:05PM +0800, Chenhui Sun wrote:
> >>The size of the updated DTB file may be increased, so we need to allocate
> >>more memory than the original DTB size,or memory overflow may happen.
> >>
> >>Contributed-under: TianoCore Contribution Agreement 1.0
> >>Signed-off-by: shaochangliang <shaochangliang@huawei.com>
> >>Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> >>Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
> >>---
> >>  Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
> >>index 8586e33..699a820 100644
> >>--- a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
> >>+++ b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
> >>@@ -112,7 +112,7 @@ EFIAPI UpdateFdt (
> >>      Size = (UINTN)fdt_totalsize ((VOID*)(PcdGet64(FdtFileAddress)));
> >>      NewFdtBlobSize = Size + ADD_FILE_LENGTH;
> >>-    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(Size), &NewFdtBlobBase);
> >>+    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(NewFdtBlobSize), &NewFdtBlobBase);
> >This is clearly a fix.
> >However, can you clarify whether
> >(VOID) CopyMem((VOID*)NewFdtBlobBase, Fdt, Size);
> >also need to be updated?
> Copy the source fdt file to NewFdtBlobBase, then update it, the size may
> increase after updating,
> so there just copy the origin data.

OK, in that case:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> thanks.
> Chenhui
> >
> >>      if (EFI_ERROR (Status))
> >>      {
> >>          return EFI_OUT_OF_RESOURCES;
> >>-- 
> >>1.9.1
> >>
>
diff mbox series

Patch

diff --git a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
index 8586e33..699a820 100644
--- a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
+++ b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c
@@ -112,7 +112,7 @@  EFIAPI UpdateFdt (
     Size = (UINTN)fdt_totalsize ((VOID*)(PcdGet64(FdtFileAddress)));
     NewFdtBlobSize = Size + ADD_FILE_LENGTH;
 
-    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(Size), &NewFdtBlobBase);
+    Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(NewFdtBlobSize), &NewFdtBlobBase);
     if (EFI_ERROR (Status))
     {
         return EFI_OUT_OF_RESOURCES;