The new BN_num_bits_word in 1.0.2o triggers bug in MS C 14.00.60131 for ARM

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

The new BN_num_bits_word in 1.0.2o triggers bug in MS C 14.00.60131 for ARM

Jakob Bohm-7
The new constant-time implementation of BN_num_bits_word in
OpenSSL 1.0.2o triggers a compiler bug in Microsoft C
14.00.60131 for ARM (part of Visual Studio 2005 SP1).  The bug
happens at all optimizations levels except completely disabled.

Unfortunately, that version of Microsoft C is the official
system compiler for some popular versions of Windows CE.

I have not seen this bug in the MS compilers in Visual Studio
2012 (CL 17.00.60610.1) and Embedded Visual C++ 4.0 (CLARM.exe
12.20.9409), but those are system compilers for different CE
versions and the system headers are not compatible across
compilers.

Here is an example of what the function is miscompiled to:

BN_num_bits_word:
   00000000: E3500000 cmp         r0, #0
   00000004: 0A000007 beq         |$LN3|
   00000008: E1A030A0 mov         r3, r0, lsr #1
   0000000C: E2633000 rsb         r3, r3, #0
   00000010: E1A03FA3 mov         r3, r3, lsr #31
   00000014: E2633000 rsb         r3, r3, #0
   00000018: E2033001 and         r3, r3, #1
   0000001C: E3A02001 mov         r2, #1
   00000020: E0830002 add         r0, r3, r2
   00000024: E1A0F00E mov         pc, lr
$LN3:
   00000028: E1A030A0 mov         r3, r0, lsr #1
   0000002C: E2633000 rsb         r3, r3, #0
   00000030: E1A03FA3 mov         r3, r3, lsr #31
   00000034: E2633000 rsb         r3, r3, #0
   00000038: E2033001 and         r3, r3, #1
   0000003C: E3A02000 mov         r2, #0
   00000040: E0830002 add         r0, r3, r2
   00000044: E1A0F00E mov         pc, lr


The patch below works around this, porting this to OpenSSL 1.1.x
is left as an exercise for the reader:

Work around a serious compiler bug in the system compiler for CE 5.00
    (Including Windows Mobile 5.x and 6.x).
Author: WiseMo A/S, licensed under the Openssl license
diff -Naur openssl-1.0.2o.orig/crypto/bn/bn_lib.c
openssl-1.0.2o/crypto/bn/bn_lib.c
--- openssl-1.0.2o.orig/crypto/bn/bn_lib.c    2018-03-27
13:54:46.000000000 +0000
+++ openssl-1.0.2o/crypto/bn/bn_lib.c    2018-05-11 10:17:55.000000000 +0000
@@ -142,6 +142,27 @@
      return (&const_one);
  }

+#if _MSC_VER >= 1400 && _MSC_VER < 1500 && \
+    (defined(__arm__)            || \
+     defined(__arm)              || \
+     defined(_M_ARM)             || \
+     defined(_ARM_)              || \
+     defined(ARM)                || \
+     defined(__ARMEL__)          || \
+     defined(__ARM_BIG_ENDIAN)   || \
+     defined(__MARM__)           || \
+     defined(__ARM_ARCH)         || \
+     defined(__thumb__)          || \
+     defined(_M_THUMB)           || \
+     defined(_THUMB_)            || \
+     defined(THUMB))
+// This compiler (The Visual Studio 2005 ARM Compiler), at least with SP1
+// completely miscompiles BN_num_bits_word() !
+#define MS_BROKEN_BN_num_bits_word  1
+
+#pragma optimize("", off)
+#endif
+
  int BN_num_bits_word(BN_ULONG l)
  {
      BN_ULONG x, mask;
@@ -197,6 +218,10 @@
      return ((i * BN_BITS2) + BN_num_bits_word(a->d[i]));
  }

+#ifdef MS_BROKEN_BN_num_bits_word
+#pragma optimize("", on)
+#endif
+
  void BN_clear_free(BIGNUM *a)
  {
      int i;



Enjoy

Jakob
--
Jakob Bohm, CIO, Partner, WiseMo A/S.  https://www.wisemo.com
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|

Re: The new BN_num_bits_word in 1.0.2o triggers bug in MS C 14.00.60131 for ARM

Kurt Roeckx
On Mon, Aug 06, 2018 at 04:30:54PM +0200, Jakob Bohm wrote:
> The patch below works around this, porting this to OpenSSL 1.1.x
> is left as an exercise for the reader:

Can you please open a pull request on github for that?


Kurt

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|

Re: The new BN_num_bits_word in 1.0.2o triggers bug in MS C 14.00.60131 for ARM

Jakob Bohm-7
On 09/08/2018 23:23, Kurt Roeckx wrote:
> On Mon, Aug 06, 2018 at 04:30:54PM +0200, Jakob Bohm wrote:
>> The patch below works around this, porting this to OpenSSL 1.1.x
>> is left as an exercise for the reader:
> Can you please open a pull request on github for that?
>
>
> Kurt
>
This may be some extra work due to the additional rules and procedures
for formal github contributions. I sent the report earlier than my
larger patch collection as it fixed a very recently introduced issue.

Could you at least treat it as a bug report until I can get the
procedural stuff ready.

P.S.

Are you considering moving to a neutral hosting solution now that
Github has been purchased by a major software publisher?

Enjoy

Jakob
--
Jakob Bohm, CIO, Partner, WiseMo A/S.  https://www.wisemo.com
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users