[v1] MdeModulePkg/CapsuleLib: Code refinements

[edk2] [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

Posted by Hao Wu 7 years, 7 months ago

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
index fc0f8698a9..191a432369 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
@@ -343,6 +343,9 @@ InitCapsuleLastVariable (
                         NULL
                         );
       }
+      if (CapsuleResult != NULL) {
+        FreePool (CapsuleResult);
+      }
     }
 
     // Lock it in normal boot path per UEFI spec.
-- 
2.12.0.windows.1

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

Re: [edk2] [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

Posted by Yao, Jiewen 7 years, 7 months ago

According to GetVariable2() API, there is no guarantee that Data will be NULL, if error is returned.

I suggest we use status check instead of data pointer check.


/**
  Returns the status whether get the variable success. The function retrieves 
  variable  through the UEFI Runtime Service GetVariable().  The 
  returned buffer is allocated using AllocatePool().  The caller is responsible
  for freeing this buffer with FreePool().

  If Name  is NULL, then ASSERT().
  If Guid  is NULL, then ASSERT().
  If Value is NULL, then ASSERT().

  @param[in]  Name  The pointer to a Null-terminated Unicode string.
  @param[in]  Guid  The pointer to an EFI_GUID structure
  @param[out] Value The buffer point saved the variable info.
  @param[out] Size  The buffer size of the variable.

  @return EFI_OUT_OF_RESOURCES      Allocate buffer failed.
  @return EFI_SUCCESS               Find the specified variable.
  @return Others Errors             Return errors from call to gRT->GetVariable.

**/



> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, March 10, 2017 2:07 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by
> GetVariable2 API
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> index fc0f8698a9..191a432369 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> @@ -343,6 +343,9 @@ InitCapsuleLastVariable (
>                          NULL
>                          );
>        }
> +      if (CapsuleResult != NULL) {
> +        FreePool (CapsuleResult);
> +      }
>      }
> 
>      // Lock it in normal boot path per UEFI spec.
> --
> 2.12.0.windows.1

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

Re: [edk2] [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by GetVariable2 API

Posted by Wu, Hao A 7 years, 7 months ago

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Friday, March 10, 2017 2:17 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Subject: RE: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned
> by GetVariable2 API
> 
> According to GetVariable2() API, there is no guarantee that Data will be NULL,
> if error is returned.
> 
> I suggest we use status check instead of data pointer check.
> 

Yes, you are right. I will send out a V2 version of the series.

Best Regards,
Hao Wu

> 
> /**
>   Returns the status whether get the variable success. The function retrieves
>   variable  through the UEFI Runtime Service GetVariable().  The
>   returned buffer is allocated using AllocatePool().  The caller is responsible
>   for freeing this buffer with FreePool().
> 
>   If Name  is NULL, then ASSERT().
>   If Guid  is NULL, then ASSERT().
>   If Value is NULL, then ASSERT().
> 
>   @param[in]  Name  The pointer to a Null-terminated Unicode string.
>   @param[in]  Guid  The pointer to an EFI_GUID structure
>   @param[out] Value The buffer point saved the variable info.
>   @param[out] Size  The buffer size of the variable.
> 
>   @return EFI_OUT_OF_RESOURCES      Allocate buffer failed.
>   @return EFI_SUCCESS               Find the specified variable.
>   @return Others Errors             Return errors from call to gRT->GetVariable.
> 
> **/
> 
> 
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Friday, March 10, 2017 2:07 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: [PATCH 2/2] MdeModulePkg/CapsuleLib: Free the buffer returned by
> > GetVariable2 API
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git
> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > index fc0f8698a9..191a432369 100644
> > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> > @@ -343,6 +343,9 @@ InitCapsuleLastVariable (
> >                          NULL
> >                          );
> >        }
> > +      if (CapsuleResult != NULL) {
> > +        FreePool (CapsuleResult);
> > +      }
> >      }
> >
> >      // Lock it in normal boot path per UEFI spec.
> > --
> > 2.12.0.windows.1

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