Re: [edk2] [PATCH v2] CryptoPkg: use correct OpenSSL #define for LP64 data model

Subject: Re: [edk2] [PATCH v2] CryptoPkg: use correct OpenSSL #define for LP64 data model

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

To: "edk2-devel@lists.sourceforge.net" <edk2-devel@lists.sourceforge.net>

Date: 2014-09-19 15:09:41

On 19 September 2014 03:38, Laszlo Ersek  wrote:
> Hi Ard,
>
> On 09/18/14 23:33, Ard Biesheuvel wrote:
>> Users of the LP64 data model should declare SIXTY_FOUR_BIT_LONG,
>> not SIXTY_FOUR_BIT when building OpenSSL.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Reviewed-By: Olivier Martin 
>> Reviewed-by: Andrew Fish 
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  CryptoPkg/Library/OpensslLib/OpensslLib.inf | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> index d380158a4339..f32afb9b65ff 100644
>> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> @@ -655,10 +655,10 @@
>>     INTEL:*_*_X64_CC_FLAGS                 = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT
>>     INTEL:*_*_IPF_CC_FLAGS                 = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT
>>     GCC:*_*_IA32_CC_FLAGS                  = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT
>> -   GCC:*_*_X64_CC_FLAGS                   = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT
>> -   GCC:*_*_IPF_CC_FLAGS                   = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT
>> +   GCC:*_*_X64_CC_FLAGS                   = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG
>> +   GCC:*_*_IPF_CC_FLAGS                   = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG
>>     GCC:*_*_ARM_CC_FLAGS                   = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT
>> -   GCC:*_*_AARCH64_CC_FLAGS               = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT
>> +   GCC:*_*_AARCH64_CC_FLAGS               = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG
>>
>>     # suppress the following warnings in openssl so we don't break the build with warnings-as-errors:
>>     # 1295: Deprecated declaration  - give arg types
>> @@ -674,4 +674,4 @@
>>     # 1296: Extended constant initialiser used
>>     RVCT:*_*_ARM_CC_FLAGS                  = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) --library_interface=aeabi_clib99 --fpu=vfpv3 -DTHIRTY_TWO_BIT --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188
>>     XCODE:*_*_IA32_CC_FLAGS                = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT
>> -   XCODE:*_*_X64_CC_FLAGS                 = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT
>> +   XCODE:*_*_X64_CC_FLAGS                 = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG
>>
>
> what's the impact of this issue?
>

The impact of this issue is that, while the code appears to work
correctly either way, you are essentially telling OpenSSL that
sizeof(long) == 4, which at the very least means you are taking
different code paths through OpenSSL than when you run the same exact
version under the OS on the same hardware (note that OpenSSL uses a
fair amount of #ifdef __GNUC__ and #ifdef  conditionals as
well). I don't think performance would be affected, i.e., the bignum
code will probably use 8-byte ints either way, but it's better to be
100% correct with issues like this.

-- 
Ard.

------------------------------------------------------------------------------
Slashdot TV.  Video for Nerds.  Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel