On 06/23/15 15:54, Peter Jones wrote:
> On Tue, Jun 23, 2015 at 03:34:25PM +0200, Laszlo Ersek 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>
>
> Yeah, EFIAPI to get the right calling conventions on x86_64 is
> definitely the right thing here. Is this what was breaking shim for
> you?
Yes. The verification of shim, as the first ever signed binary loaded
during boot, consists of several steps, and one of those steps checks DB
for a direct signature (or cert). The very first attempt to validate the
binary fails -- of course a later, alternative, step *would* succeed --,
but when that first attempt fails, the OpenSSL code that tries to store
an error message, using the function being fixed here, blows up.
So everything is fine on my end with this patch in place. The error
message is now successfully stored (and then thrown away silently), and
shim is accepted / verified same as before in the later step.
> It's mildly annoying that EFIAPI is pretty much 100% about ABI calling
> conventions and has nothing to do with actual API, but that ship has
> long past sailed.
>
> Acked-by: Peter Jones <pjo...@redhat.com>
Thanks! :)
Laszlo
>> ---
>> .../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
