1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v1 0/3] Fix possible uninitialized uses
  4. View patch

[edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use

Sergei Dmitrouk posted 3 patches 4 years, 9 months ago
There is a newer version of this series
[edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by Sergei Dmitrouk 4 years, 9 months ago 
`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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75119): https://edk2.groups.io/g/devel/message/75119
Mute This Topic: https://groups.io/mt/82822574/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by Yao, Jiewen 4 years, 9 months ago 
Hi Sergei
Thank you very much for the fix.
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

I am a little surprised why it is not caught before. It is an obvious logic issue.

Do you think we can do anything on CI, to catch it during pre-check-in in the future?
I just feel it is burden to make it post-check-in fix.


Thank you
Yao Jiewen

> -----Original Message-----
> From: Sergei Dmitrouk <sergei@posteo.net>
> Sent: Friday, May 14, 2021 8:17 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
> 
> `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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75135): https://edk2.groups.io/g/devel/message/75135
Mute This Topic: https://groups.io/mt/82822574/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by Sergei Dmitrouk 4 years, 9 months ago 
Hello Jiewen,

I get the error only for GCC49 and not for GCC5 toolchain.  CI uses GCC5.

So I compared build commands and this seems to depend on LTO.  Adding `-flto`
impedes compiler's ability to detect such simple issues.

I've found relevant bug report, there is even fix suggestion from last month:

    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844

Regards,
Sergei

On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> Hi Sergei
> Thank you very much for the fix.
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>
> I am a little surprised why it is not caught before. It is an obvious logic issue.
>
> Do you think we can do anything on CI, to catch it during pre-check-in in the future?
> I just feel it is burden to make it post-check-in fix.
>
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: Sergei Dmitrouk <sergei@posteo.net>
> > Sent: Friday, May 14, 2021 8:17 PM
> > To: devel@edk2.groups.io
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
> >
> > `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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75138): https://edk2.groups.io/g/devel/message/75138
Mute This Topic: https://groups.io/mt/82822574/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by gaoliming 4 years, 8 months ago 
Sergei:
  Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool chain.
They both work on the different GCC version, such as gcc5, gcc6..

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
-ffat-lto-objects option that can trig the warning with LTO option. Do you
try it?

  If this option works, we can update GCC5 tool chain definition in
tools_def.txt, then this issue can be detected in CI GCC5 build. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> Dmitrouk
> 发送时间: 2021年5月15日 21:01
> 收件人: devel@edk2.groups.io; jiewen.yao@intel.com
> 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> 主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> uninitialized use
> 
> Hello Jiewen,
> 
> I get the error only for GCC49 and not for GCC5 toolchain.  CI uses GCC5.
> 
> So I compared build commands and this seems to depend on LTO.  Adding
> `-flto`
> impedes compiler's ability to detect such simple issues.
> 
> I've found relevant bug report, there is even fix suggestion from last
month:
> 
>     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844
> 
> Regards,
> Sergei
> 
> On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> > Hi Sergei
> > Thank you very much for the fix.
> > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> >
> > I am a little surprised why it is not caught before. It is an obvious
logic issue.
> >
> > Do you think we can do anything on CI, to catch it during pre-check-in
in the
> future?
> > I just feel it is burden to make it post-check-in fix.
> >
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: Sergei Dmitrouk <sergei@posteo.net>
> > > Sent: Friday, May 14, 2021 8:17 PM
> > > To: devel@edk2.groups.io
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>;
> > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> <guomin.jiang@intel.com>
> > > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
uninitialized
> use
> > >
> > > `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
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75230): https://edk2.groups.io/g/devel/message/75230
Mute This Topic: https://groups.io/mt/82902007/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by Ard Biesheuvel 4 years, 8 months ago 
Please merge this fix asap. Our CI is broken because of it, and we are
in the soft freeze so we need the CI up and running to catch potential
issues before the release.

Thanks,
Ard.

On Tue, 18 May 2021 at 02:59, gaoliming <gaoliming@byosoft.com.cn> wrote:
>
> Sergei:
>   Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool chain.
> They both work on the different GCC version, such as gcc5, gcc6..
>
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
> -ffat-lto-objects option that can trig the warning with LTO option. Do you
> try it?
>
>   If this option works, we can update GCC5 tool chain definition in
> tools_def.txt, then this issue can be detected in CI GCC5 build.
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> > Dmitrouk
> > 发送时间: 2021年5月15日 21:01
> > 收件人: devel@edk2.groups.io; jiewen.yao@intel.com
> > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > 主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > uninitialized use
> >
> > Hello Jiewen,
> >
> > I get the error only for GCC49 and not for GCC5 toolchain.  CI uses GCC5.
> >
> > So I compared build commands and this seems to depend on LTO.  Adding
> > `-flto`
> > impedes compiler's ability to detect such simple issues.
> >
> > I've found relevant bug report, there is even fix suggestion from last
> month:
> >
> >     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844
> >
> > Regards,
> > Sergei
> >
> > On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> > > Hi Sergei
> > > Thank you very much for the fix.
> > > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > >
> > > I am a little surprised why it is not caught before. It is an obvious
> logic issue.
> > >
> > > Do you think we can do anything on CI, to catch it during pre-check-in
> in the
> > future?
> > > I just feel it is burden to make it post-check-in fix.
> > >
> > >
> > > Thank you
> > > Yao Jiewen
> > >
> > > > -----Original Message-----
> > > > From: Sergei Dmitrouk <sergei@posteo.net>
> > > > Sent: Friday, May 14, 2021 8:17 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>;
> > > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> > <guomin.jiang@intel.com>
> > > > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> uninitialized
> > use
> > > >
> > > > `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
> >
> >
> >
> >
>
>
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75240): https://edk2.groups.io/g/devel/message/75240
Mute This Topic: https://groups.io/mt/82906677/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by Wang, Jian J 4 years, 8 months ago 
Ard,

Patch 1&2 haven't got r-b. I'm not sure we can merge patch 3 separately.

Regards,
Jian

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Tuesday, May 18, 2021 3:27 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Liming Gao (Byosoft address)
> <gaoliming@byosoft.com.cn>
> Cc: sergei@posteo.net; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> <guomin.jiang@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> uninitialized use
> 
> Please merge this fix asap. Our CI is broken because of it, and we are
> in the soft freeze so we need the CI up and running to catch potential
> issues before the release.
> 
> Thanks,
> Ard.
> 
> On Tue, 18 May 2021 at 02:59, gaoliming <gaoliming@byosoft.com.cn> wrote:
> >
> > Sergei:
> >   Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool chain.
> > They both work on the different GCC version, such as gcc5, gcc6..
> >
> >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
> > -ffat-lto-objects option that can trig the warning with LTO option. Do you
> > try it?
> >
> >   If this option works, we can update GCC5 tool chain definition in
> > tools_def.txt, then this issue can be detected in CI GCC5 build.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> > > Dmitrouk
> > > 发送时间: 2021年5月15日 21:01
> > > 收件人: devel@edk2.groups.io; jiewen.yao@intel.com
> > > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > > 主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > > uninitialized use
> > >
> > > Hello Jiewen,
> > >
> > > I get the error only for GCC49 and not for GCC5 toolchain.  CI uses GCC5.
> > >
> > > So I compared build commands and this seems to depend on LTO.  Adding
> > > `-flto`
> > > impedes compiler's ability to detect such simple issues.
> > >
> > > I've found relevant bug report, there is even fix suggestion from last
> > month:
> > >
> > >     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844
> > >
> > > Regards,
> > > Sergei
> > >
> > > On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> > > > Hi Sergei
> > > > Thank you very much for the fix.
> > > > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > > >
> > > > I am a little surprised why it is not caught before. It is an obvious
> > logic issue.
> > > >
> > > > Do you think we can do anything on CI, to catch it during pre-check-in
> > in the
> > > future?
> > > > I just feel it is burden to make it post-check-in fix.
> > > >
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > > > -----Original Message-----
> > > > > From: Sergei Dmitrouk <sergei@posteo.net>
> > > > > Sent: Friday, May 14, 2021 8:17 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>;
> > > > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> > > <guomin.jiang@intel.com>
> > > > > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > uninitialized
> > > use
> > > > >
> > > > > `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
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> > 
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75241): https://edk2.groups.io/g/devel/message/75241
Mute This Topic: https://groups.io/mt/82906677/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by gaoliming 4 years, 8 months ago 
Jian:
  These three patches are separate. They don't impact others. So, I think we can merge single one. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Wang, Jian J <jian.j.wang@intel.com>
> 发送时间: 2021年5月18日 15:36
> 收件人: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io
> <devel@edk2.groups.io>; Liming Gao (Byosoft address)
> <gaoliming@byosoft.com.cn>
> 抄送: sergei@posteo.net; Yao, Jiewen <jiewen.yao@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> 主题: RE: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> uninitialized use
> 
> Ard,
> 
> Patch 1&2 haven't got r-b. I'm not sure we can merge patch 3 separately.
> 
> Regards,
> Jian
> 
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Sent: Tuesday, May 18, 2021 3:27 PM
> > To: edk2-devel-groups-io <devel@edk2.groups.io>; Liming Gao (Byosoft
> address)
> > <gaoliming@byosoft.com.cn>
> > Cc: sergei@posteo.net; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang,
> Guomin
> > <guomin.jiang@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix
> possible
> > uninitialized use
> >
> > Please merge this fix asap. Our CI is broken because of it, and we are
> > in the soft freeze so we need the CI up and running to catch potential
> > issues before the release.
> >
> > Thanks,
> > Ard.
> >
> > On Tue, 18 May 2021 at 02:59, gaoliming <gaoliming@byosoft.com.cn>
> wrote:
> > >
> > > Sergei:
> > >   Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool
> chain.
> > > They both work on the different GCC version, such as gcc5, gcc6..
> > >
> > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
> > > -ffat-lto-objects option that can trig the warning with LTO option. Do you
> > > try it?
> > >
> > >   If this option works, we can update GCC5 tool chain definition in
> > > tools_def.txt, then this issue can be detected in CI GCC5 build.
> > >
> > > Thanks
> > > Liming
> > > > -----邮件原件-----
> > > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> > > > Dmitrouk
> > > > 发送时间: 2021年5月15日 21:01
> > > > 收件人: devel@edk2.groups.io; jiewen.yao@intel.com
> > > > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > > > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > > > 主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix
> possible
> > > > uninitialized use
> > > >
> > > > Hello Jiewen,
> > > >
> > > > I get the error only for GCC49 and not for GCC5 toolchain.  CI uses
> GCC5.
> > > >
> > > > So I compared build commands and this seems to depend on LTO.
> Adding
> > > > `-flto`
> > > > impedes compiler's ability to detect such simple issues.
> > > >
> > > > I've found relevant bug report, there is even fix suggestion from last
> > > month:
> > > >
> > > >     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844
> > > >
> > > > Regards,
> > > > Sergei
> > > >
> > > > On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> > > > > Hi Sergei
> > > > > Thank you very much for the fix.
> > > > > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > > > >
> > > > > I am a little surprised why it is not caught before. It is an obvious
> > > logic issue.
> > > > >
> > > > > Do you think we can do anything on CI, to catch it during pre-check-in
> > > in the
> > > > future?
> > > > > I just feel it is burden to make it post-check-in fix.
> > > > >
> > > > >
> > > > > Thank you
> > > > > Yao Jiewen
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Sergei Dmitrouk <sergei@posteo.net>
> > > > > > Sent: Friday, May 14, 2021 8:17 PM
> > > > > > To: devel@edk2.groups.io
> > > > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > > <jian.j.wang@intel.com>;
> > > > > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> > > > <guomin.jiang@intel.com>
> > > > > > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > > uninitialized
> > > > use
> > > > > >
> > > > > > `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
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > > 
> > >
> > >




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75291): https://edk2.groups.io/g/devel/message/75291
Mute This Topic: https://groups.io/mt/82926553/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by Yao, Jiewen 4 years, 8 months ago 
OK. I suggest we merge this specific one ASAP, since it is blocking other development work.


> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Wednesday, May 19, 2021 8:59 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; 'Ard Biesheuvel' <ardb@kernel.org>;
> 'edk2-devel-groups-io' <devel@edk2.groups.io>
> Cc: sergei@posteo.net; Yao, Jiewen <jiewen.yao@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> uninitialized use
> 
> Jian:
>   These three patches are separate. They don't impact others. So, I think we can
> merge single one.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Wang, Jian J <jian.j.wang@intel.com>
> > 发送时间: 2021年5月18日 15:36
> > 收件人: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io
> > <devel@edk2.groups.io>; Liming Gao (Byosoft address)
> > <gaoliming@byosoft.com.cn>
> > 抄送: sergei@posteo.net; Yao, Jiewen <jiewen.yao@intel.com>; Lu, XiaoyuX
> > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > 主题: RE: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > uninitialized use
> >
> > Ard,
> >
> > Patch 1&2 haven't got r-b. I'm not sure we can merge patch 3 separately.
> >
> > Regards,
> > Jian
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > > Sent: Tuesday, May 18, 2021 3:27 PM
> > > To: edk2-devel-groups-io <devel@edk2.groups.io>; Liming Gao (Byosoft
> > address)
> > > <gaoliming@byosoft.com.cn>
> > > Cc: sergei@posteo.net; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang,
> > Guomin
> > > <guomin.jiang@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix
> > possible
> > > uninitialized use
> > >
> > > Please merge this fix asap. Our CI is broken because of it, and we are
> > > in the soft freeze so we need the CI up and running to catch potential
> > > issues before the release.
> > >
> > > Thanks,
> > > Ard.
> > >
> > > On Tue, 18 May 2021 at 02:59, gaoliming <gaoliming@byosoft.com.cn>
> > wrote:
> > > >
> > > > Sergei:
> > > >   Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool
> > chain.
> > > > They both work on the different GCC version, such as gcc5, gcc6..
> > > >
> > > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
> > > > -ffat-lto-objects option that can trig the warning with LTO option. Do you
> > > > try it?
> > > >
> > > >   If this option works, we can update GCC5 tool chain definition in
> > > > tools_def.txt, then this issue can be detected in CI GCC5 build.
> > > >
> > > > Thanks
> > > > Liming
> > > > > -----邮件原件-----
> > > > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> > > > > Dmitrouk
> > > > > 发送时间: 2021年5月15日 21:01
> > > > > 收件人: devel@edk2.groups.io; jiewen.yao@intel.com
> > > > > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > > > > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > > > > 主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix
> > possible
> > > > > uninitialized use
> > > > >
> > > > > Hello Jiewen,
> > > > >
> > > > > I get the error only for GCC49 and not for GCC5 toolchain.  CI uses
> > GCC5.
> > > > >
> > > > > So I compared build commands and this seems to depend on LTO.
> > Adding
> > > > > `-flto`
> > > > > impedes compiler's ability to detect such simple issues.
> > > > >
> > > > > I've found relevant bug report, there is even fix suggestion from last
> > > > month:
> > > > >
> > > > >     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844
> > > > >
> > > > > Regards,
> > > > > Sergei
> > > > >
> > > > > On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> > > > > > Hi Sergei
> > > > > > Thank you very much for the fix.
> > > > > > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > > > > >
> > > > > > I am a little surprised why it is not caught before. It is an obvious
> > > > logic issue.
> > > > > >
> > > > > > Do you think we can do anything on CI, to catch it during pre-check-in
> > > > in the
> > > > > future?
> > > > > > I just feel it is burden to make it post-check-in fix.
> > > > > >
> > > > > >
> > > > > > Thank you
> > > > > > Yao Jiewen
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Sergei Dmitrouk <sergei@posteo.net>
> > > > > > > Sent: Friday, May 14, 2021 8:17 PM
> > > > > > > To: devel@edk2.groups.io
> > > > > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > > > <jian.j.wang@intel.com>;
> > > > > > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> > > > > <guomin.jiang@intel.com>
> > > > > > > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > > > uninitialized
> > > > > use
> > > > > > >
> > > > > > > `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
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > 
> > > >
> > > >
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75293): https://edk2.groups.io/g/devel/message/75293
Mute This Topic: https://groups.io/mt/82926630/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: 回复: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by Sergei Dmitrouk 4 years, 8 months ago 
Yes, adding `-ffat-lto-objects` makes the warning appear even with GCC 5.5.0.

Regards,
Sergei

On Tue, May 18, 2021 at 08:59:05AM +0800, gaoliming wrote:
> Sergei:
>   Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool chain.
> They both work on the different GCC version, such as gcc5, gcc6..
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
> -ffat-lto-objects option that can trig the warning with LTO option. Do you
> try it?
> 
>   If this option works, we can update GCC5 tool chain definition in
> tools_def.txt, then this issue can be detected in CI GCC5 build. 
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> > Dmitrouk
> > 发送时间: 2021年5月15日 21:01
> > 收件人: devel@edk2.groups.io; jiewen.yao@intel.com
> > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > 主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > uninitialized use
> > 
> > Hello Jiewen,
> > 
> > I get the error only for GCC49 and not for GCC5 toolchain.  CI uses GCC5.
> > 
> > So I compared build commands and this seems to depend on LTO.  Adding
> > `-flto`
> > impedes compiler's ability to detect such simple issues.
> > 
> > I've found relevant bug report, there is even fix suggestion from last
> month:
> > 
> >     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844
> > 
> > Regards,
> > Sergei
> > 
> > On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> > > Hi Sergei
> > > Thank you very much for the fix.
> > > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > >
> > > I am a little surprised why it is not caught before. It is an obvious
> logic issue.
> > >
> > > Do you think we can do anything on CI, to catch it during pre-check-in
> in the
> > future?
> > > I just feel it is burden to make it post-check-in fix.
> > >
> > >
> > > Thank you
> > > Yao Jiewen
> > >
> > > > -----Original Message-----
> > > > From: Sergei Dmitrouk <sergei@posteo.net>
> > > > Sent: Friday, May 14, 2021 8:17 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>;
> > > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> > <guomin.jiang@intel.com>
> > > > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> uninitialized
> > use
> > > >
> > > > `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
> > 
> > 
> > 
> > 
> 
> 
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75260): https://edk2.groups.io/g/devel/message/75260
Mute This Topic: https://groups.io/mt/82902007/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: 回复: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use
Posted by gaoliming 4 years, 8 months ago 
Sergei:
  Thanks for your confirmation. I will send the patch to update GCC5 tool chain with this option. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> Dmitrouk
> 发送时间: 2021年5月19日 0:03
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: jiewen.yao@intel.com; 'Wang, Jian J' <jian.j.wang@intel.com>; 'Lu,
> XiaoyuX' <xiaoyux.lu@intel.com>; 'Jiang, Guomin' <guomin.jiang@intel.com>
> 主题: Re: 回复: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix
> possible uninitialized use
> 
> Yes, adding `-ffat-lto-objects` makes the warning appear even with GCC 5.5.0.
> 
> Regards,
> Sergei
> 
> On Tue, May 18, 2021 at 08:59:05AM +0800, gaoliming wrote:
> > Sergei:
> >   Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool chain.
> > They both work on the different GCC version, such as gcc5, gcc6..
> >
> >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
> > -ffat-lto-objects option that can trig the warning with LTO option. Do you
> > try it?
> >
> >   If this option works, we can update GCC5 tool chain definition in
> > tools_def.txt, then this issue can be detected in CI GCC5 build.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
> > > Dmitrouk
> > > 发送时间: 2021年5月15日 21:01
> > > 收件人: devel@edk2.groups.io; jiewen.yao@intel.com
> > > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > > <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
> > > 主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix
> possible
> > > uninitialized use
> > >
> > > Hello Jiewen,
> > >
> > > I get the error only for GCC49 and not for GCC5 toolchain.  CI uses GCC5.
> > >
> > > So I compared build commands and this seems to depend on LTO.  Adding
> > > `-flto`
> > > impedes compiler's ability to detect such simple issues.
> > >
> > > I've found relevant bug report, there is even fix suggestion from last
> > month:
> > >
> > >     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844
> > >
> > > Regards,
> > > Sergei
> > >
> > > On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
> > > > Hi Sergei
> > > > Thank you very much for the fix.
> > > > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > > >
> > > > I am a little surprised why it is not caught before. It is an obvious
> > logic issue.
> > > >
> > > > Do you think we can do anything on CI, to catch it during pre-check-in
> > in the
> > > future?
> > > > I just feel it is burden to make it post-check-in fix.
> > > >
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > > > -----Original Message-----
> > > > > From: Sergei Dmitrouk <sergei@posteo.net>
> > > > > Sent: Friday, May 14, 2021 8:17 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>;
> > > > > Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> > > <guomin.jiang@intel.com>
> > > > > Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
> > uninitialized
> > > use
> > > > >
> > > > > `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
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75299): https://edk2.groups.io/g/devel/message/75299
Mute This Topic: https://groups.io/mt/82927068/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.