On 06/23/15 16:16, Long, Qin wrote: > Reviewed-by: Qin Long <qin.l...@intel.com>
Thanks for the reviews. Since this issue breaks secure boot builds, generally speaking, and because the pkg maintainer reviewed the patch, I committed it at once: SVN r17689. I hope that Gary won't mind my doing that before his feedback. (In retrospect the fix should be quite non-controversial; it just restores two earlier hunks, with changed pathnames.) Thanks! Laszlo > Best Regards & Thanks, > LONG, Qin > > -----Original Message----- > From: Long, Qin [mailto:qin.l...@intel.com] > Sent: Tuesday, June 23, 2015 10:12 PM > To: Ard Biesheuvel; Laszlo Ersek > Cc: edk2-devel@lists.sourceforge.net > Subject: Re: [edk2] [PATCH] CryptoPkg: OpensslLib: reintroduce EFIAPI for > ERR_add_error_data() > > Laszlo, > > Yes, the EFIAPI definition for ERR_add_error_data() was missed in this > updated patch file. > It's cool. Thanks for catching this. > > > Best Regards & Thanks, > LONG, Qin > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Tuesday, June 23, 2015 10:07 PM > To: Laszlo Ersek > Cc: edk2-devel@lists.sourceforge.net; Long, Qin; Gary Ching-Pang Lin; Peter > Jones > Subject: Re: [PATCH] CryptoPkg: OpensslLib: reintroduce EFIAPI for > ERR_add_error_data() > > On 23 June 2015 at 15:34, Laszlo Ersek <ler...@redhat.com> wrote: >> >> Git commit f93f78ea70 (SVN r17633), with subject "CryptoPkg: Update >> openssl patch file from 0.9.8zf to 1.0.2c", replaced the file >> >> CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch >> >> with >> >> CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2c.patch >> >> In the process, two hunks were lost that used to add EFIAPI to the >> declaration of the variadic function ERR_add_error_data(). >> >> The VA_START() macro, from "MdePkg/Include/Base.h", expands to an >> EFIAPI-dependent implementation when >> >> !defined(__CC_ARM) && (!defined(__GNUC__) || >> defined(NO_BUILTIN_VA_FUNCS)) >> >> Under such circumstances, the va_start() macro invocation in >> ERR_add_error_data() -- which is translated to VA_START() by >> "CryptoPkg/Include/OpenSslSupport.h" -- results in EFIAPI-dependent >> code, but callers of the function pass the arguments incorrectly, >> because the declaration doesn't state EFIAPI. >> >> This leads to crashes when ERR_add_error_vdata(), called by >> ERR_add_error_data(), tries to access the arguments forwarded to it. >> >> Restore the missing hunk from before SVN r17633. >> >> Cc: Qin Long <qin.l...@intel.com> >> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> >> Cc: Gary Ching-Pang Lin <g...@suse.com> >> Cc: Peter Jones <pjo...@redhat.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> > > Great that you were able to track this down. It also explains why I didn't > see any of this on AArch64, since we don't have this EFIAPI issue, > fortunately. > > Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > > >> --- >> .../Library/OpensslLib/EDKII_openssl-1.0.2c.patch | 34 >> ++++++++++++++++++++++ >> 1 file changed, 34 insertions(+) >> >> diff --git a/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2c.patch >> b/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2c.patch >> index 54e14d8..0d9575e 100644 >> --- a/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2c.patch >> +++ b/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2c.patch >> @@ -344,3 +344,37 @@ diff U3 crypto/opensslconf.h crypto/opensslconf.h >> #endif >> >> #if defined(HEADER_RC4_LOCL_H) && !defined(CONFIG_HEADER_RC4_LOCL_H) >> +diff U3 crypto/err/err.c crypto/err/err.c >> +--- crypto/err/err.c >> ++++ crypto/err/err.c >> +@@ -1072,7 +1072,12 @@ void ERR_set_error_data(char *data, int flags) >> + es->err_data_flags[i] = flags; >> + } >> + >> ++/* Add EFIAPI for UEFI version. */ >> ++#if defined(OPENSSL_SYS_UEFI) >> ++void EFIAPI ERR_add_error_data(int num, ...) #else >> + void ERR_add_error_data(int num, ...) >> ++#endif >> + { >> + va_list args; >> + va_start(args, num); >> +diff U3 crypto/err/err.h crypto/err/err.h >> +--- crypto/err/err.h >> ++++ crypto/err/err.h >> +@@ -344,7 +344,14 @@ void ERR_print_errors_fp(FILE *fp); # ifndef >> +OPENSSL_NO_BIO void ERR_print_errors(BIO *bp); # endif >> ++ >> ++/* Add EFIAPI for UEFI version. */ >> ++#if defined(OPENSSL_SYS_UEFI) >> ++void EFIAPI ERR_add_error_data(int num, ...); #else >> + void ERR_add_error_data(int num, ...); >> ++#endif >> ++ >> + void ERR_add_error_vdata(int num, va_list args); void >> + ERR_load_strings(int lib, ERR_STRING_DATA str[]); void >> + ERR_unload_strings(int lib, ERR_STRING_DATA str[]); >> -- >> 1.8.3.1 >> > ------------------------------------------------------------------------------ > Monitor 25 network devices or servers for free with OpManager! > OpManager is web-based network management software that monitors network > devices and physical & virtual servers, alerts via email & sms for fault. > Monitor 25 devices for free with no restriction. Download now > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
