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

["Ard Biesheuvel" <ardb@kernel.org>]

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
> >
> >
> >
> >
>
>
>
>
>
> 
>
>