[edk2-devel] [PATCH v4 0/7] CryptoPkg: Upgrade OpenSSL to 1.1.1b
Laszlo Ersek
lersek at redhat.com
Fri May 17 13:08:11 UTC 2019
On 05/17/19 12:12, Lu, XiaoyuX wrote:
> Hi, Lerszlo:
well... I agree that my first name may not be trivial to spell, but you
can always use the clipboard, whenever in doubt.
For the record, it's "Laszlo".
>
> (1):
>
>> Unfortunately, I've found another build issue with this series. (My apologies that I didn't discover it earlier.) It is reported in the 32-bit (ARM) build of the ArmVirtQemu platform:
>>
>> CryptoPkg/Library/OpensslLib/openssl/crypto/rand/drbg_lib.c:1028:
>> undefined reference to `__aeabi_ui2d'
>>
>
> OpensslLib[Crypto].inf contains ArmSoftFloatLib as dependent library.
>
> In ArmSoftFloatLib:
>
> softfloat-for-gcc.h|98| #define uint32_to_float64 __floatunsidf
> softfloat-for-gcc.h|222| #define __floatunsidf __aeabi_ui2d
>
> softfloat-for-gcc.h|128| #define float64_to_uint32_round_to_zero __fixunsdfsi
> softfloat-for-gcc.h|234| #define __fixunsdfsi __aeabi_d2uiz
>
> But *uint32_to_float64* and *float64_to_uint32_round_to_zero* aren't implemented in softfloat.c
>
> If these two functions implement, the build will pass. (I use dummy functions and try)
See my response to Jian on this.
> (2):
>
>> thus, preferably, a CryptoPkg patch series should be at least build tested (if not boot tested) for all arches, before being posted to the mailing list.
>
> I should test ARM, since IA32 arch has Intrinsic problem(_ftol2). It is very likely that ARM arch does not support it either.
>
>> (Yes, CI would help a lot with such issues.)
>
> Now I don't have a CI environment here.
> I will setup one for building OvmfPkg, ArmVirtPkg, EmulatorPkg.
Sorry, I was unclear: I meant a community-level, central CI. Not a
personal one. And, the central CI is undergoing design discussions right
now.
Thanks
Laszlo
> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of Laszlo Ersek
> Sent: Friday, May 17, 2019 2:26 AM
> To: devel at edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu at intel.com>
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Ye, Ting <ting.ye at intel.com>; Ard Biesheuvel <ard.biesheuvel at linaro.org>; Leif Lindholm <leif.lindholm at linaro.org>
> Subject: Re: [edk2-devel] [PATCH v4 0/7] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>
> Hi,
>
> (+ Ard and Leif)
>
> On 05/16/19 09:54, Xiaoyu lu wrote:
>> This series is also available at:
>> https://github.com/xiaoyuxlu/edk2/tree/bz_1089_upgrade_to_openssl_1_1_
>> 1b_v4
>>
>> Changes:
>>
>> (1) CryptoPkgOpensslLib: Modify process_files.pl for upgrading
>> OpenSSL
>>
>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
>> crypto/store/* are excluded.
>> crypto/rand/randfile.c is excluded.
>>
>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol
>> issue
>>
>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
>> Disable warnings for buiding OpenSSL_1_1_1b
>>
>> (5) CryptoPkg/OpensslLib: Fix cross-build problem for AARCH64
>>
>> (6) CryptoPkg: Upgrade OpenSSL to 1.1.1b
>> The biggest change is use TSC as entropy source
>> If TSC isn't avaiable, fallback to TimerLib(PerformanceCounter).
>>
>> (7) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
>>
>>
>> Verification done for this series:
>> * Https boot in OvmfPkg.
>> * BaseCrypt Library test. (Ovmf, EmulatorPkg)
>>
>> Important notice:
>> Nt32Pkg doesn't support TimerLib
>>> TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemp
>>> TimerLib|late.inf
>> So it will failed in Nt32Pkg.
>>
>> Cc: Jian J Wang <jian.j.wang at intel.com>
>> Cc: Ting Ye <ting.ye at intel.com>
>>
>> Laszlo Ersek (1):
>> CryptoPkg/OpensslLib: Fix cross-build problem for AARCH64
>>
>> Xiaoyu Lu (6):
>> CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL
>> CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
>> CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue
>> CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
>> CryptoPkg: Upgrade OpenSSL to 1.1.1b
>> CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
>>
>> CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf | 4 +-
>> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 76 ++++-
>> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 67 ++++-
>> CryptoPkg/Library/Include/CrtLibSupport.h | 13 +-
>> CryptoPkg/Library/Include/openssl/opensslconf.h | 54 +++-
>> CryptoPkg/Library/Include/sys/syscall.h | 11 +
>> CryptoPkg/Library/OpensslLib/buildinf.h | 2 +
>> CryptoPkg/Library/OpensslLib/rand_pool_noise.h | 29 ++
>> CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c | 8 +-
>> .../Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 9 +-
>> .../Library/BaseCryptLib/Hmac/CryptHmacSha256.c | 8 +-
>> CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c | 22 ++
>> CryptoPkg/Library/OpensslLib/ossl_store.c | 17 ++
>> CryptoPkg/Library/OpensslLib/rand_pool.c | 316 +++++++++++++++++++++
>> CryptoPkg/Library/OpensslLib/rand_pool_noise.c | 29 ++
>> CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c | 43 +++
>> CryptoPkg/Library/OpensslLib/openssl | 2 +-
>> CryptoPkg/Library/OpensslLib/process_files.pl | 11 +-
>> 18 files changed, 669 insertions(+), 52 deletions(-) create mode
>> 100644 CryptoPkg/Library/Include/sys/syscall.h
>> create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.h
>> create mode 100644 CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c
>> create mode 100644 CryptoPkg/Library/OpensslLib/ossl_store.c
>> create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool.c
>> create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise.c
>> create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c
>>
>
> Unfortunately, I've found another build issue with this series. (My apologies that I didn't discover it earlier.) It is reported in the 32-bit (ARM) build of the ArmVirtQemu platform:
>
> CryptoPkg/Library/OpensslLib/openssl/crypto/rand/drbg_lib.c:1028:
> undefined reference to `__aeabi_ui2d'
>
> The referenced line is from the drbg_add() function:
>
> if (buflen < seedlen || randomness < (double) seedlen) {
>
> Beyond the failure to resolve the "__aeabi_ui2d" symbol, the edk2 coding style spec says, "Floating point operations are not recommended in UEFI firmware." (Even though the UEFI spec describes the required floating point environment for all architectures.)
>
> So, I'm not sure what we should do here. If we think that floating point is plain evil in edk2, then we cannot rebase edk2 to OpenSSL-1.1.1b.
>
> ... Hmmm, this seems to be the 32-bit ARM variant of [PATCH v4 3/7]!
>
> If we find floating point generally acceptable in edk2, then Ard and Leif could help us decide please whether this 32-bit ARM issue should be fixed during the feature freeze (when fixes are still allowed), or if it justifies postponing OpenSSL 1.1.1b to the next edk2 stable tag.
>
> Again, I'm sorry that I found this only now -- but "CryptoPkg/CryptoPkg.dsc" is multi-arch:
>
> SUPPORTED_ARCHITECTURES = IA32|X64|ARM|AARCH64
>
> thus, preferably, a CryptoPkg patch series should be at least build tested (if not boot tested) for all arches, before being posted to the mailing list.
>
> (Yes, CI would help a lot with such issues.)
>
> Thanks
> Laszlo
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40916): https://edk2.groups.io/g/devel/message/40916
Mute This Topic: https://groups.io/mt/31638503/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list