public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: "Wang, Jian J" <jian.j.wang@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Lu, XiaoyuX" <xiaoyux.lu@intel.com>
Cc: "Ye, Ting" <ting.ye@intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	"Gao, Liming" <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH v4 0/7] CryptoPkg: Upgrade OpenSSL to 1.1.1b
Date: Fri, 17 May 2019 15:04:42 +0200	[thread overview]
Message-ID: <0a6b50d4-3837-a5e6-7f3a-36386c65d42b@redhat.com> (raw)
In-Reply-To: <D827630B58408649ACB04F44C5100036258FB83E@SHSMSX107.ccr.corp.intel.com>

On 05/17/19 07:11, Wang, Jian J wrote:
> Hi Laszlo,
> 
> There's already a float library used in OpensslLib.inf. 
> 
> [LibraryClasses.ARM]
>   ArmSoftFloatLib
> 
> The problem is that the below instance doesn't implement __aeabi_ui2d
> and __aeabi_d2uiz (I encountered this one as well)
> 
>   ArmPkg\Library\ArmSoftFloatLib\ArmSoftFloatLib.inf
> 
> I think we can update this library support those two APIs. So what about
> we still push the patch and file a BZ to fix this issue?

I'm OK with that, but it will break ARM and AARCH64 platforms that
consume OpensslLib (directly or through BaseCryptLib), so this question
is up to Leif and Ard to decide.

Thanks
Laszlo

>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, May 17, 2019 2:26 AM
>> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>; Ard
>> Biesheuvel <ard.biesheuvel@linaro.org>; Leif Lindholm
>> <leif.lindholm@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/BaseTimerLibNullTemplat
>> e.inf
>>> So it will failed in Nt32Pkg.
>>>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>> Cc: Ting Ye <ting.ye@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


  reply	other threads:[~2019-05-17 13:04 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-16  7:54 [PATCH v4 0/7] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu
2019-05-16  7:54 ` [PATCH v4 1/7] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
2019-05-16  7:54 ` [PATCH v4 2/7] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu
2019-05-16 15:51   ` [edk2-devel] " Laszlo Ersek
2019-05-16  7:54 ` [PATCH v4 3/7] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu
2019-05-16  7:54 ` [PATCH v4 4/7] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu
2019-05-16  7:54 ` [PATCH v4 5/7] CryptoPkg/OpensslLib: Fix cross-build problem for AARCH64 Xiaoyu lu
2019-05-16 15:58   ` [edk2-devel] " Laszlo Ersek
2019-05-16  7:54 ` [PATCH v4 6/7] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu
2019-05-16 16:31   ` [edk2-devel] " Laszlo Ersek
2019-05-17 11:14     ` Xiaoyu Lu
2019-05-17 13:15       ` Laszlo Ersek
2019-05-18  7:16         ` Xiaoyu Lu
2019-05-16  7:54 ` [PATCH v4 7/7] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu
2019-05-16 18:25 ` [edk2-devel] [PATCH v4 0/7] CryptoPkg: Upgrade OpenSSL to 1.1.1b Laszlo Ersek
2019-05-17  5:11   ` Wang, Jian J
2019-05-17 13:04     ` Laszlo Ersek [this message]
2019-05-17 13:16       ` Laszlo Ersek
2019-05-17 15:06         ` Ard Biesheuvel
2019-05-20  1:40           ` Wang, Jian J
     [not found]           ` <15A0408CA29C0595.820@groups.io>
2019-05-21  7:43             ` Wang, Jian J
2019-05-21  9:01               ` Ard Biesheuvel
2019-05-21  9:09                 ` Wang, Jian J
2019-05-21 12:23                   ` Laszlo Ersek
2019-05-21 13:02                     ` Wang, Jian J
2019-05-21 13:34                       ` Laszlo Ersek
2019-05-21 13:39                     ` Ard Biesheuvel
2019-05-23  5:10                       ` Wang, Jian J
2019-05-17 10:12   ` Xiaoyu Lu
2019-05-17 13:08     ` Laszlo Ersek
2019-05-18  7:37       ` Xiaoyu Lu
2019-05-16 18:53 ` Laszlo Ersek
2019-05-17  5:00   ` [edk2-devel] " Wang, Jian J
2019-05-17  9:17 ` Gary Lin
2019-05-18  7:26   ` Xiaoyu Lu
2019-05-20  1:48     ` Gary Lin
2019-05-21 21:14 ` Laszlo Ersek
2019-05-22  0:10   ` Michael D Kinney
2019-05-22  9:05     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a6b50d4-3837-a5e6-7f3a-36386c65d42b@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox