From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.nue.novell.com (smtp.nue.novell.com [195.135.221.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D60B2803D2 for ; Wed, 22 Mar 2017 03:11:46 -0700 (PDT) Received: from emea4-mta.ukb.novell.com ([10.120.13.87]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Wed, 22 Mar 2017 11:11:45 +0100 Received: from GaryWorkstation (nwb-a10-snat.microfocus.com [10.120.13.201]) by emea4-mta.ukb.novell.com with ESMTP (TLS encrypted); Wed, 22 Mar 2017 10:11:16 +0000 Date: Wed, 22 Mar 2017 18:11:10 +0800 From: Gary Lin To: Qin Long Cc: edk2-devel@lists.01.org, ard.biesheuvel@linaro.org, ting.ye@intel.com, David Woodhouse , ronald.cron@arm.com, jiaxin.wu@intel.com, lersek@redhat.com Message-ID: <20170322101110.d7lpswvlhdpeaetd@GaryWorkstation> References: <20170321155612.1192-1-qin.long@intel.com> <20170321155612.1192-4-qin.long@intel.com> MIME-Version: 1.0 In-Reply-To: <20170321155612.1192-4-qin.long@intel.com> User-Agent: Mutt/1.6.2 (2016-07-01) Subject: Re: [PATCH v1 3/9] CryptoPkg: Fix handling of &strcmp function pointers X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Mar 2017 10:11:47 -0000 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Tue, Mar 21, 2017 at 11:56:06PM +0800, Qin Long wrote: > In a couple of places, OpenSSL code uses the address of the strcmp() > function, and assigns it to another comparator function pointer. > > Unfortunately, this falls foul of the inconsistent function ABI that we > use in EDKII. We '#define strcmp AsciiStrCmp' but AsciiStrCmp is an > EFIAPI function with the Microsoft ABI. And we're assigning its address > to a non-EFIAPI function, which may well have a different ABI. > > The compiler *should* have complained about this error, thus: > > …/crypto/objects/o_names.c: In function ‘OBJ_NAME_new_index’: > …/crypto/objects/o_names.c:94:30: error: assignment from incompatible > pointer type [-Werror=incompatible-pointer-types] > name_funcs->cmp_func = OPENSSL_strcmp; > ^ > There's another one in crypto/lhash/lhash.c::lh_new() which has an > explicit cast so even with compiler warnings we wouldn't have seen it. > > Fix this by providing an actual strcmp() function in the default ABI. > We already *had* a prototype for it in OpenSslSupport.h, which was then > superseded by the #define strcmp AsciiStrCmp. > > Now, OpenSSL code *can* use &strcmp without problems. With this patch, we probably can remove "defined(OPENSSL_SYS_UEFI)" from https://github.com/openssl/openssl/blob/master/crypto/objects/o_names.c#L32 Gary Lin > > Cc: Ting Ye > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Gary Lin > Cc: Ronald Cron > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: David Woodhouse > Signed-off-by: Qin Long > --- > CryptoPkg/Include/OpenSslSupport.h | 3 +-- > CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c | 8 +++++++- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/CryptoPkg/Include/OpenSslSupport.h b/CryptoPkg/Include/OpenSslSupport.h > index 91567c78f8..c3c5b5dcd7 100644 > --- a/CryptoPkg/Include/OpenSslSupport.h > +++ b/CryptoPkg/Include/OpenSslSupport.h > @@ -1,7 +1,7 @@ > /** @file > Root include file to support building OpenSSL Crypto Library. > > -Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > @@ -275,7 +275,6 @@ extern FILE *stdout; > #define memchr(buf,ch,count) ScanMem8(buf,(UINTN)(count),(UINT8)ch) > #define memcmp(buf1,buf2,count) (int)(CompareMem(buf1,buf2,(UINTN)(count))) > #define memmove(dest,source,count) CopyMem(dest,source,(UINTN)(count)) > -#define strcmp AsciiStrCmp > #define strncmp(string1,string2,count) (int)(AsciiStrnCmp(string1,string2,(UINTN)(count))) > #define strcpy(strDest,strSource) AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource) > #define strncpy(strDest,strSource,count) AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count) > diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c > index 9d6867ebce..e8a76d07ff 100644 > --- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c > +++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c > @@ -2,7 +2,7 @@ > Intrinsic Memory Routines Wrapper Implementation for OpenSSL-based > Cryptographic Library. > > -Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > @@ -15,6 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > #include > #include > +#include > > /* OpenSSL will use floating point support, and C compiler produces the _fltused > symbol by default. Simply define this symbol here to satisfy the linker. */ > @@ -44,3 +45,8 @@ void * memset (void *dest, char ch, unsigned int count) > > return dest; > } > + > +int strcmp (const char *s1, const char *s2) > +{ > + return (int)AsciiStrCmp(s1, s2); > +} > -- > 2.11.1.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel