public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer
@ 2018-05-08 11:46 Dandan Bi
  2018-05-09  2:31 ` Gary Lin
  0 siblings, 1 reply; 3+ messages in thread
From: Dandan Bi @ 2018-05-08 11:46 UTC (permalink / raw)
  To: edk2-devel; +Cc: Eric Dong, Liming Gao

_CLEAR_SAVED_OPHDR () is used for initialize the variables.
We should not update it to free memory.
It will cause some pointer used before initialization.
This patch is to fix this issue.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrSyntax.g | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 4b0a43606ea..cc042ab4307 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -4103,12 +4103,15 @@ vfrStatementExpression [UINT32 RootLevel, UINT32 ExpOpCount = 0] :
                                                               }
                                                             }
                                                           }
                                                           
                                                           if ($RootLevel == 0) {
-                                                            _CLEAR_SAVED_OPHDR ();
-                                                            mCIfrOpHdrIndex --;
+                                                            if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
+                                                              delete mCIfrOpHdr[mCIfrOpHdrIndex];
+                                                              mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
+                                                            }
+                                                             mCIfrOpHdrIndex --;
                                                           }
                                                        >>
   ;
 
 //
@@ -5082,14 +5085,11 @@ EfiVfrParser::_SAVE_OPHDR_COND (
 VOID
 EfiVfrParser::_CLEAR_SAVED_OPHDR (
   VOID
   )
 {
-  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
-    delete mCIfrOpHdr[mCIfrOpHdrIndex];
-    mCIfrOpHdr[mCIfrOpHdrIndex]     = NULL;
-  }
+  mCIfrOpHdr[mCIfrOpHdrIndex]       = NULL;
   mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;
 }
 
 BOOLEAN
 EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
-- 
2.14.3.windows.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer
  2018-05-08 11:46 [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer Dandan Bi
@ 2018-05-09  2:31 ` Gary Lin
  2018-05-09  5:09   ` Bi, Dandan
  0 siblings, 1 reply; 3+ messages in thread
From: Gary Lin @ 2018-05-09  2:31 UTC (permalink / raw)
  To: Dandan Bi; +Cc: edk2-devel, Eric Dong, Liming Gao

On Tue, May 08, 2018 at 07:46:19PM +0800, Dandan Bi wrote:
> _CLEAR_SAVED_OPHDR () is used for initialize the variables.
> We should not update it to free memory.
> It will cause some pointer used before initialization.
> This patch is to fix this issue.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 4b0a43606ea..cc042ab4307 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -4103,12 +4103,15 @@ vfrStatementExpression [UINT32 RootLevel, UINT32 ExpOpCount = 0] :
>                                                                }
>                                                              }
>                                                            }
>                                                            
>                                                            if ($RootLevel == 0) {
> -                                                            _CLEAR_SAVED_OPHDR ();
> -                                                            mCIfrOpHdrIndex --;
> +                                                            if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> +                                                              delete mCIfrOpHdr[mCIfrOpHdrIndex];
> +                                                              mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> +                                                            }

> +                                                             mCIfrOpHdrIndex --;
An extra space was added.

>                                                            }
>                                                         >>
>    ;
>  
>  //
> @@ -5082,14 +5085,11 @@ EfiVfrParser::_SAVE_OPHDR_COND (
>  VOID
>  EfiVfrParser::_CLEAR_SAVED_OPHDR (
>    VOID
>    )
>  {
> -  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> -    delete mCIfrOpHdr[mCIfrOpHdrIndex];
> -    mCIfrOpHdr[mCIfrOpHdrIndex]     = NULL;
> -  }
> +  mCIfrOpHdr[mCIfrOpHdrIndex]       = NULL;
>    mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;
>  }
>  
>  BOOLEAN
>  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
> -- 
I applied the patch and triggered the rebuild of ovmf. It's now built on
all versions and arch.

Thanks for fixing it.

Tested-by: Gary Lin <glin@suse.com>

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer
  2018-05-09  2:31 ` Gary Lin
@ 2018-05-09  5:09   ` Bi, Dandan
  0 siblings, 0 replies; 3+ messages in thread
From: Bi, Dandan @ 2018-05-09  5:09 UTC (permalink / raw)
  To: Gary Lin; +Cc: edk2-devel@lists.01.org, Dong, Eric, Gao, Liming

Thanks for your test work. 
I have created a V2 patch which make the code logic more clean. Would you mind to try the V2 patch in your environment?


Thanks,
Dandan

-----Original Message-----
From: Gary Lin [mailto:glin@suse.com] 
Sent: Wednesday, May 9, 2018 10:32 AM
To: Bi, Dandan <dandan.bi@intel.com>
Cc: edk2-devel@lists.01.org; Dong, Eric <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer

On Tue, May 08, 2018 at 07:46:19PM +0800, Dandan Bi wrote:
> _CLEAR_SAVED_OPHDR () is used for initialize the variables.
> We should not update it to free memory.
> It will cause some pointer used before initialization.
> This patch is to fix this issue.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 4b0a43606ea..cc042ab4307 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -4103,12 +4103,15 @@ vfrStatementExpression [UINT32 RootLevel, UINT32 ExpOpCount = 0] :
>                                                                }
>                                                              }
>                                                            }
>                                                            
>                                                            if ($RootLevel == 0) {
> -                                                            _CLEAR_SAVED_OPHDR ();
> -                                                            mCIfrOpHdrIndex --;
> +                                                            if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> +                                                              delete mCIfrOpHdr[mCIfrOpHdrIndex];
> +                                                              mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> +                                                            }

> +                                                             
> + mCIfrOpHdrIndex --;
An extra space was added.

>                                                            }
>                                                         >>
>    ;
>  
>  //
> @@ -5082,14 +5085,11 @@ EfiVfrParser::_SAVE_OPHDR_COND (  VOID  
> EfiVfrParser::_CLEAR_SAVED_OPHDR (
>    VOID
>    )
>  {
> -  if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> -    delete mCIfrOpHdr[mCIfrOpHdrIndex];
> -    mCIfrOpHdr[mCIfrOpHdrIndex]     = NULL;
> -  }
> +  mCIfrOpHdr[mCIfrOpHdrIndex]       = NULL;
>    mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;  }
>  
>  BOOLEAN
>  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
> --
I applied the patch and triggered the rebuild of ovmf. It's now built on all versions and arch.

Thanks for fixing it.

Tested-by: Gary Lin <glin@suse.com>

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-09  5:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-08 11:46 [patch] BaseTools/VfrCompile: Avoid using uninitialized pointer Dandan Bi
2018-05-09  2:31 ` Gary Lin
2018-05-09  5:09   ` Bi, Dandan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox