[PATCH v2 1/2] MdeModulePkg Variable: Fix a corner case issue about setting a variable

[PATCH v2 1/2] MdeModulePkg Variable: Fix a corner case issue about setting a variable

[cinnamon shia]

Fix the issue that failed to update or add a UEFI variable if the remaining size is equal to the data size
of the variable.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6caf603b3d..7303681aaa 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -17,7 +17,7 @@
   integer overflow. It should also check attribute to avoid authentication bypass.
 
 Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+(C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -288,7 +288,7 @@ UpdateVariableStore (
       DataPtr += mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase;
     }
 
-    if ((DataPtr + DataSize) >= ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
+    if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
       return EFI_INVALID_PARAMETER;
     }
   } else {
@@ -301,7 +301,7 @@ UpdateVariableStore (
       DataPtr += mVariableModuleGlobal->VariableGlobal.VolatileVariableBase;
     }
 
-    if ((DataPtr + DataSize) >= ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
+    if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
       return EFI_INVALID_PARAMETER;
     }
 
-- 
2.16.1.windows.4

[PATCH v2 2/2] MdeModulePkg Variable: Fix the returned status in UpdateVariableStore

[cinnamon shia]

If Fvb is a NULL, return EFI_UNSUPPORTED.
If the remaining size is not enough, return EFI_OUT_OF_RESOURCES.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 7303681aaa..d17361f805 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -238,6 +238,8 @@ IsValidVariableHeader (
   @param Buffer                  Pointer to the buffer from which data is written.
 
   @retval EFI_INVALID_PARAMETER  Parameters not valid.
+  @retval EFI_UNSUPPORTED        Fvb is a NULL for Non-Volatile variable update.
+  @retval EFI_OUT_OF_RESOURCES   The remaining size is not enough.
   @retval EFI_SUCCESS            Variable store successfully updated.
 
 **/
@@ -274,7 +276,7 @@ UpdateVariableStore (
   //
   if (!Volatile) {
     if (Fvb == NULL) {
-      return EFI_INVALID_PARAMETER;
+      return EFI_UNSUPPORTED;
     }
     Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr);
     ASSERT_EFI_ERROR (Status);
@@ -289,7 +291,7 @@ UpdateVariableStore (
     }
 
     if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
-      return EFI_INVALID_PARAMETER;
+      return EFI_OUT_OF_RESOURCES;
     }
   } else {
     //
@@ -302,7 +304,7 @@ UpdateVariableStore (
     }
 
     if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
-      return EFI_INVALID_PARAMETER;
+      return EFI_OUT_OF_RESOURCES;
     }
 
     //
-- 
2.16.1.windows.4

Re: [PATCH v2 1/2] MdeModulePkg Variable: Fix a corner case issue about setting a variable

[Laszlo Ersek]

Hi Cinnamon,

On 05/11/18 17:21, cinnamon shia wrote:
> Fix the issue that failed to update or add a UEFI variable if the remaining size is equal to the data size
> of the variable.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
> Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 6caf603b3d..7303681aaa 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -17,7 +17,7 @@
>    integer overflow. It should also check attribute to avoid authentication bypass.
>  
>  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> -(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
> +(C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD License
>  which accompanies this distribution.  The full text of the license may be found at
> @@ -288,7 +288,7 @@ UpdateVariableStore (
>        DataPtr += mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase;
>      }
>  
> -    if ((DataPtr + DataSize) >= ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
> +    if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
>        return EFI_INVALID_PARAMETER;
>      }
>    } else {
> @@ -301,7 +301,7 @@ UpdateVariableStore (
>        DataPtr += mVariableModuleGlobal->VariableGlobal.VolatileVariableBase;
>      }
>  
> -    if ((DataPtr + DataSize) >= ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
> +    if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
>        return EFI_INVALID_PARAMETER;
>      }
>  
> 

the series looks OK to me on a quick skim, but I prefer that the
MdeModulePkg maintainers review this, if that's OK with you.

Thanks
Laszlo

Re: [PATCH v2 1/2] MdeModulePkg Variable: Fix a corner case issue about setting a variable

[Zeng, Star]

Thanks for the contribution.
Reviewed-by: Star Zeng <star.zeng@intel.com> to the patch series.
And push the patches to d741d1419350dc4ae868d4a9396f9da33a3416a0 and 989f7a2cf0e27123fda5ca538b15832e115e0f4e.


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of cinnamon shia
Sent: Friday, May 11, 2018 11:21 PM
To: edk2-devel@lists.01.org
Cc: lersek@redhat.com; Zeng, Star <star.zeng@intel.com>; Ansen Huang <ansen.huang@hpe.com>
Subject: [edk2] [PATCH v2 1/2] MdeModulePkg Variable: Fix a corner case issue about setting a variable

Fix the issue that failed to update or add a UEFI variable if the remaining size is equal to the data size
of the variable.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6caf603b3d..7303681aaa 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -17,7 +17,7 @@
   integer overflow. It should also check attribute to avoid authentication bypass.
 
 Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+(C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -288,7 +288,7 @@ UpdateVariableStore (
       DataPtr += mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase;
     }
 
-    if ((DataPtr + DataSize) >= ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
+    if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
       return EFI_INVALID_PARAMETER;
     }
   } else {
@@ -301,7 +301,7 @@ UpdateVariableStore (
       DataPtr += mVariableModuleGlobal->VariableGlobal.VolatileVariableBase;
     }
 
-    if ((DataPtr + DataSize) >= ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
+    if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
       return EFI_INVALID_PARAMETER;
     }
 
-- 
2.16.1.windows.4

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