1. Patchew
  2. EDK2
  3. [edk2] [PATCH v2 00/11] Upgrade CryptoPkg to use the OpenSSL 1.1.0xx/stable release
  4. View patch

[edk2] [PATCH v2 06/11] CryptoPkg: Fix handling of &strcmp function pointers

Qin Long posted 11 patches 7 years, 7 months ago
[edk2] [PATCH v2 06/11] CryptoPkg: Fix handling of &strcmp function pointers
Posted by Qin Long 7 years, 7 months ago 
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.

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.

Cc: Ting Ye <ting.ye@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Lin <glin@suse.com>
Cc: Ronald Cron <ronald.cron@arm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Qin Long <qin.long@intel.com>
---
 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 e011a7cfee..e6858a94a4 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.<BR>
+Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
 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
@@ -276,7 +276,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.<BR>
+Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
 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 <Base.h>
 #include <Library/BaseMemoryLib.h>
+#include <Library/BaseLib.h>
 
 /* 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
Re: [edk2] [PATCH v2 06/11] CryptoPkg: Fix handling of &strcmp function pointers
Posted by Laszlo Ersek 7 years, 7 months ago 
On 03/23/17 14:19, 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.
> 
> 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.
> 
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Lin <glin@suse.com>
> Cc: Ronald Cron <ronald.cron@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Qin Long <qin.long@intel.com>
> ---
>  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 e011a7cfee..e6858a94a4 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.<BR>
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  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
> @@ -276,7 +276,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.<BR>
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  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 <Base.h>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/BaseLib.h>
>  
>  /* 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);
> +}
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

(Tested as described in
<http://mid.mail-archive.com/4410fa53-b0e4-d64a-7b95-8a430a4c7b06@redhat.com>.)

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 06/11] CryptoPkg: Fix handling of &strcmp function pointers
Posted by Gary Lin 7 years, 7 months ago 
On Thu, Mar 23, 2017 at 09:19:27PM +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.
> 
> 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.
> 
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Lin <glin@suse.com>
> Cc: Ronald Cron <ronald.cron@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Qin Long <qin.long@intel.com>

I tested OVMF and it works for me.

Tested-by: Gary Lin <glin@suse.com>

Gary Lin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.