[PATCH v1 0/3] Fix possible uninitialized uses

[PATCH v1 0/3] Fix possible uninitialized uses

[Sergei Dmitrouk]

v1:

Compiling for IA32 target with gcc-5.5.0 emits "maybe-uninitialized" warnings.
Compilation command: build -a IA32 -p OvmfPkg/OvmfPkgIa32.dsc -t GCC49

Unlike other cases mentioned in
https://bugzilla.tianocore.org/show_bug.cgi?id=3228
these seem to be actual issues in the code.  Read patches for specifics.

v2:

Second patch was simplified.

Sergei Dmitrouk (3):
  ShellPkg/HttpDynamicCommand: Fix possible uninitialized use
  MdeModulePkg/PciBusDxe: Fix possible uninitialized use
  CryptoPkg/BaseCryptLib: Fix possible uninitialized use

 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c     | 1 +
 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c             | 5 ++---
 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c   | 1 +
 4 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.17.6

[PATCH v2 1/3] ShellPkg/HttpDynamicCommand: Fix possible uninitialized use

[Sergei Dmitrouk]

`Status` can be used uninitialized:

    /* Evaluates to FALSE */
    if (ShellGetExecutionBreakFlag ()) {
        Status = EFI_ABORTED;
        break;
    }

    /* Evaluates to FALSE */
    if (!Context->ContentDownloaded && !Context->ResponseToken.Event) {
        Status = ...;
        ASSERT_EFI_ERROR (Status);
    } else {
        ResponseMessage.Data.Response = NULL;
    }

    /* UNINITIALIZED USE */
    if (EFI_ERROR (Status)) {
        break;
    }

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Sergei Dmitrouk <sergei@posteo.net>
---
 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
index 3735a4a7e645..7b9b2d238015 100644
--- a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
@@ -1524,6 +1524,7 @@ GetResponse (
   Context->ResponseToken.Message = &ResponseMessage;
   Context->ContentLength = 0;
   Context->Status = REQ_OK;
+  Status = EFI_SUCCESS;
   MsgParser = NULL;
   ResponseData.StatusCode = HTTP_STATUS_UNSUPPORTED_STATUS;
   ResponseMessage.Data.Response = &ResponseData;
-- 
2.17.6

回复: [edk2-devel] [PATCH v2 1/3] ShellPkg/HttpDynamicCommand: Fix possible uninitialized use

[gaoliming]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

This fix is clear. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> Dmitrouk
> 发送时间: 2021年5月19日 0:10
> 收件人: devel@edk2.groups.io
> 抄送: Ray Ni <ray.ni@intel.com>; Zhichao Gao <zhichao.gao@intel.com>
> 主题: [edk2-devel] [PATCH v2 1/3] ShellPkg/HttpDynamicCommand: Fix
> possible uninitialized use
> 
> `Status` can be used uninitialized:
> 
>     /* Evaluates to FALSE */
>     if (ShellGetExecutionBreakFlag ()) {
>         Status = EFI_ABORTED;
>         break;
>     }
> 
>     /* Evaluates to FALSE */
>     if (!Context->ContentDownloaded && !Context->ResponseToken.Event) {
>         Status = ...;
>         ASSERT_EFI_ERROR (Status);
>     } else {
>         ResponseMessage.Data.Response = NULL;
>     }
> 
>     /* UNINITIALIZED USE */
>     if (EFI_ERROR (Status)) {
>         break;
>     }
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Sergei Dmitrouk <sergei@posteo.net>
> ---
>  ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> index 3735a4a7e645..7b9b2d238015 100644
> --- a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> +++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> @@ -1524,6 +1524,7 @@ GetResponse (
>    Context->ResponseToken.Message = &ResponseMessage;
>    Context->ContentLength = 0;
>    Context->Status = REQ_OK;
> +  Status = EFI_SUCCESS;
>    MsgParser = NULL;
>    ResponseData.StatusCode = HTTP_STATUS_UNSUPPORTED_STATUS;
>    ResponseMessage.Data.Response = &ResponseData;
> --
> 2.17.6
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v2 1/3] ShellPkg/HttpDynamicCommand: Fix possible uninitialized use

[Gao, Zhichao]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> gaoliming
> Sent: Wednesday, May 19, 2021 9:13 AM
> To: devel@edk2.groups.io; sergei@posteo.net
> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v2 1/3] ShellPkg/HttpDynamicCommand:
> Fix possible uninitialized use
> 
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> 
> This fix is clear.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> Dmitrouk
> > 发送时间: 2021年5月19日 0:10
> > 收件人: devel@edk2.groups.io
> > 抄送: Ray Ni <ray.ni@intel.com>; Zhichao Gao <zhichao.gao@intel.com>
> > 主题: [edk2-devel] [PATCH v2 1/3] ShellPkg/HttpDynamicCommand: Fix
> > possible uninitialized use
> >
> > `Status` can be used uninitialized:
> >
> >     /* Evaluates to FALSE */
> >     if (ShellGetExecutionBreakFlag ()) {
> >         Status = EFI_ABORTED;
> >         break;
> >     }
> >
> >     /* Evaluates to FALSE */
> >     if (!Context->ContentDownloaded && !Context->ResponseToken.Event)
> {
> >         Status = ...;
> >         ASSERT_EFI_ERROR (Status);
> >     } else {
> >         ResponseMessage.Data.Response = NULL;
> >     }
> >
> >     /* UNINITIALIZED USE */
> >     if (EFI_ERROR (Status)) {
> >         break;
> >     }
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: Sergei Dmitrouk <sergei@posteo.net>
> > ---
> >  ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> > b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> > index 3735a4a7e645..7b9b2d238015 100644
> > --- a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> > +++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> > @@ -1524,6 +1524,7 @@ GetResponse (
> >    Context->ResponseToken.Message = &ResponseMessage;
> >    Context->ContentLength = 0;
> >    Context->Status = REQ_OK;
> > +  Status = EFI_SUCCESS;
> >    MsgParser = NULL;
> >    ResponseData.StatusCode = HTTP_STATUS_UNSUPPORTED_STATUS;
> >    ResponseMessage.Data.Response = &ResponseData;
> > --
> > 2.17.6
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 
> 
> 

[PATCH v2 2/3] MdeModulePkg/PciBusDxe: Fix possible uninitialized use

[Sergei Dmitrouk]

If the function gets invalid value for the `ResizableBarOp` parameter
and asserts are disabled, `Bit` can be used uninitialized.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Sergei Dmitrouk <sergei@posteo.net>
---

Notes:
    v2:
    - simplify if-statement to avoid unused branches

 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 6bba28367165..4caac56f1dcd 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -1778,10 +1778,9 @@ PciProgramResizableBar (
 
     if (ResizableBarOp == PciResizableBarMax) {
       Bit = HighBitSet64(Capabilities);
-    } else if (ResizableBarOp == PciResizableBarMin) {
+    } else {
+      ASSERT (ResizableBarOp == PciResizableBarMin);
       Bit = LowBitSet64(Capabilities);
-    } else {
-      ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp == PciResizableBarMin));
     }
 
     ASSERT (Bit >= 0);
-- 
2.17.6

Re: [PATCH v2 2/3] MdeModulePkg/PciBusDxe: Fix possible uninitialized use

[Wu, Hao A]

> -----Original Message-----
> From: Sergei Dmitrouk <sergei@posteo.net>
> Sent: Wednesday, May 19, 2021 12:10 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v2 2/3] MdeModulePkg/PciBusDxe: Fix possible uninitialized
> use
> 
> If the function gets invalid value for the `ResizableBarOp` parameter and
> asserts are disabled, `Bit` can be used uninitialized.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Sergei Dmitrouk <sergei@posteo.net>
> ---
> 
> Notes:
>     v2:
>     - simplify if-statement to avoid unused branches
> 
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> index 6bba28367165..4caac56f1dcd 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> @@ -1778,10 +1778,9 @@ PciProgramResizableBar (
> 
>      if (ResizableBarOp == PciResizableBarMax) {
>        Bit = HighBitSet64(Capabilities);
> -    } else if (ResizableBarOp == PciResizableBarMin) {
> +    } else {
> +      ASSERT (ResizableBarOp == PciResizableBarMin);
>        Bit = LowBitSet64(Capabilities);
> -    } else {
> -      ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp ==
> PciResizableBarMin));


Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


>      }
> 
>      ASSERT (Bit >= 0);
> --
> 2.17.6

[PATCH v2 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use

[Sergei Dmitrouk]

`Result` can be used uninitialized in both functions after following
either first or second `goto` statement.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Signed-off-by: Sergei Dmitrouk <sergei@posteo.net>
---
 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c     | 1 +
 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
index 4009d37d5f91..0b2960f06c4c 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
@@ -82,6 +82,7 @@ RsaPssVerify (
   EVP_PKEY_CTX *KeyCtx;
   CONST EVP_MD  *HashAlg;
 
+  Result = FALSE;
   EvpRsaKey = NULL;
   EvpVerifyCtx = NULL;
   KeyCtx = NULL;
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
index b66b6f7296ad..ece765f9ae0a 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
@@ -97,6 +97,7 @@ RsaPssSign (
   EVP_PKEY_CTX          *KeyCtx;
   CONST EVP_MD          *HashAlg;
 
+  Result = FALSE;
   EvpRsaKey = NULL;
   EvpVerifyCtx = NULL;
   KeyCtx = NULL;
-- 
2.17.6

[PATCH v1 0/3] Fix possible uninitialized uses

[Sergei Dmitrouk]

Compiling for IA32 target with gcc-5.5.0 emits "maybe-uninitialized" warnings.
Compilation command: build -a IA32 -p OvmfPkg/OvmfPkgIa32.dsc -t GCC49

Unlike other cases mentioned in
https://bugzilla.tianocore.org/show_bug.cgi?id=3228
these seem to be actual issues in the code.  Read patches for specifics.

Sergei Dmitrouk (3):
  ShellPkg/HttpDynamicCommand: Fix possible uninitialized use
  MdeModulePkg/PciBusDxe: Fix possible uninitialized use
  CryptoPkg/BaseCryptLib: Fix possible uninitialized use

 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c     | 1 +
 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c             | 5 +++++
 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c   | 1 +
 4 files changed, 8 insertions(+)

-- 
2.17.6