artful openssl FTBFS on armhf

[Marc Deslauriers]

openssl FTBFS on artful armhf with the following:

../util/shlib_wrap.sh ./sha256t
Testing SHA-256
TEST 1 of 3 failed.

[Marc Deslauriers (mdeslaur)]

It FTBFS with gcc 7.2 in artful and bionic, but builds fine with gcc 7.1 that was previously in artful.

Openssl 1.0.2m fails in the same way.

[Chris Coulson (chrisccoulson)]

I stepped through 2 builds side-by-side in gdb - one good build built with gcc 7.1, and one bad build, built with gcc 7.2. I managed to narrow it down to a bug in sha256_block_data_order.

One of the first differences I spotted was that the good build branches almost immediately to a NEON code path (sha256_block_data_order_neon), whereas the broken build continues on the non-NEON code path.

If we look at the first few instructions of sha256_block_data_order in a good build:

   0xf7699c60 <+0>: sub r3, pc, #8
   0xf7699c64 <+4>: ldr r12, [pc, #-40] ; 0xf7699c44
   0xf7699c68 <+8>: ldr r12, [r3, r12]

The first instruction basically loads the address of the start of the function in to %r3, which we can see if we step past it:

(gdb) info registers
r0 0x413558 4273496
r1 0x413580 4273536
r2 0x1 1
r3 0xf7699c60 4150893664
r4 0x413558 4273496
r5 0xfffef35c 4294898524
r6 0x0 0
r7 0x413580 4273536
r8 0x0 0
r9 0xf77efab8 4152294072
r10 0xf77b9dec 4152073708
r11 0x0 0
r12 0x0 0
sp 0xfffef2c8 0xfffef2c8
lr 0xf7697e5c -144081316
pc 0xf7699c64 0xf7699c64 <sha256_block_data_order+4>
cpsr 0x80080010 -2146959344
(gdb) p sha256_block_data_order
$1 = {<text variable, no debug info>} 0xf7699c60 <sha256_block_data_order>

The second instruction loads a value from an address 40 bytes before the instruction in to %r12. Looking in sha256-armv4.S, this value is "OPENSSL_armcap_P - sha256_block_data_order", or the offset of OPENSSL_armcap_P from the start of sha256_block_data_order.
The third instruction loads the value of OPENSSL_armcap_P in to %r12.

Stepping through these instructions gives this state:

(gdb) info registers
r0 0x413558 4273496
r1 0x413580 4273536
r2 0x1 1
r3 0xf7699c60 4150893664
r4 0x413558 4273496
r5 0xfffef35c 4294898524
r6 0x0 0
r7 0x413580 4273536
r8 0x0 0
r9 0xf77efab8 4152294072
r10 0xf77b9dec 4152073708
r11 0x0 0
r12 0x3 3
sp 0xfffef2c8 0xfffef2c8
lr 0xf7697e5c -144081316
pc 0xf7699c6c 0xf7699c6c <sha256_block_data_order+12>
cpsr 0x80080010 -2146959344

So the value of OPENSSL_armcap_P is 3, which causes the following instructions to take the NEON path:

   0xf7699c6c <+12>: tst r12, #16
   0xf7699c70 <+16>: bne 0xf769b660 <sha256_block_data_order_armv8>
   0xf7699c74 <+20>: tst r12, #1
   0xf7699c78 <+24>: bne 0xf769aa60 <sha256_block_data_order_neon>

[Chris Coulson (chrisccoulson)]

On the broken build, the first thing to notice is that when entering sha256_block_data_order, we are running in Thumb mode, as bit 5 of the status register is set:

(gdb) info registers
r0 0x4b7558 4945240
r1 0x4b7580 4945280
r2 0x1 1
r3 0x0 0
r4 0x4b7558 4945240
r5 0xfffef44c 4294898764
r6 0x0 0
r7 0x4b7580 4945280
r8 0x3 3
r9 0xfffef44c 4294898764
r10 0x0 0
r11 0x0 0
r12 0x0 0
sp 0xfffef3b8 0xfffef3b8
lr 0x414387 4277127
pc 0x4160c0 0x4160c0 <sha256_block_data_order>
cpsr 0x40080030 1074266160

The good build was not running in Thumb mode. sha256-armv4.S enables Thumb instructions if __thumb2__ is defined by the compiler, see:

.text
#if __ARM_ARCH__<7
.code 32
#else
.syntax unified
# ifdef __thumb2__
.thumb
# else
.code 32
# endif
#endif

Sure enough, there's a difference between the 2 tested gcc builds.

Bad build:
$ gcc -dM -E - < /dev/null | grep __thumb2__
#define __thumb2__ 1
$

Good build:
$ gcc -dM -E - < /dev/null | grep __thumb2__
$

Looking in the changelog for gcc-7, I see the following comment for 7.2.0-2:
  * Restore configuring with --with-mode=thumb on armhf. Closes: #873584.

[Chris Coulson (chrisccoulson)]

Continuing debugging the broken build, if we look at the first few instructions of sha256_block_data_order:

   0x004160c0 <+0>: subw r3, pc, #3
   0x004160c4 <+4>: ldr.w r12, [pc, #-36] ; 0x4160a4
   0x004160c8 <+8>: ldr.w r12, [r3, r12]

This looks similar to before - the intention being that %r3 is loaded with the address of the start of the function, %r12 is loaded with the offset of OPENSSL_armcap_P from the start of the function, then %r12 is loaded with the value of OPENSSL_armcap_P.

However, the first instruction is broken - the #3 should be a #4. Stepping past it produces this state:

(gdb) info registers
r0 0x4b7558 4945240
r1 0x4b7580 4945280
r2 0x1 1
r3 0x4160c1 4284609
r4 0x4b7558 4945240
r5 0xfffef44c 4294898764
r6 0x0 0
r7 0x4b7580 4945280
r8 0x3 3
r9 0xfffef44c 4294898764
r10 0x0 0
r11 0x0 0
r12 0x0 0
sp 0xfffef3b8 0xfffef3b8
lr 0x414387 4277127
pc 0x4160c4 0x4160c4 <sha256_block_data_order+4>
cpsr 0x40080030 1074266160
(gdb) p sha256_block_data_order
$1 = {<text variable, no debug info>} 0x4160c0 <sha256_block_data_order>

Now, %r3 is the address of the start of the function, off-by-1. Continuing to step through demonstrates that we load the wrong value in to %r12, and this causes the non-NEON code path to be executed, which somehow doesn't work.

If I step past these first 3 instructions, and then write the expected value of OPENSSL_armcap_P in to %r12, the test completes successfully:

(gdb) info registers
r0 0x4b7558 4945240
r1 0x4b7580 4945280
r2 0x1 1
r3 0x4160c1 4284609
r4 0x4b7558 4945240
r5 0xfffef44c 4294898764
r6 0x0 0
r7 0x4b7580 4945280
r8 0x3 3
r9 0xfffef44c 4294898764
r10 0x0 0
r11 0x0 0
r12 0x1000000 16777216
sp 0xfffef3b8 0xfffef3b8
lr 0x414387 4277127
pc 0x4160cc 0x4160cc <sha256_block_data_order+12>
cpsr 0x40080030 1074266160
(gdb) set $r12 = 3
(gdb) info registers
r0 0x4b7558 4945240
r1 0x4b7580 4945280
r2 0x1 1
r3 0x4160c1 4284609
r4 0x4b7558 4945240
r5 0xfffef44c 4294898764
r6 0x0 0
r7 0x4b7580 4945280
r8 0x3 3
r9 0xfffef44c 4294898764
r10 0x0 0
r11 0x0 0
r12 0x3 3
sp 0xfffef3b8 0xfffef3b8
lr 0x414387 4277127
pc 0x4160cc 0x4160cc <sha256_block_data_order+12>
cpsr 0x40080030 1074266160
(gdb) cont
Continuing.
Testing SHA-256 .
Breakpoint 1, EVP_Digest (data=0x47f38c, count=56, md=0xfffef44c "\272x\026\277\217\001\317\352AA@\336]\256\"#\260\003a\243\226\027z\234\264\020\377a\362", size=0x0, type=0x4aff00 <sha256_md>, impl=0x0)
    at diges...

[Chris Coulson (chrisccoulson)]

The incorrect instruction at the start of sha256_block_data_order comes from the ADR pseudo-instruction in sha256-armv4.S:

.global sha256_block_data_order
.type sha256_block_data_order,%function
sha256_block_data_order:
#if __ARM_ARCH__<7
        sub r3,pc,#8 @ sha256_block_data_order
#else
        adr r3,sha256_block_data_order
#endif

The ADR instruction assembles to a SUB in our case, and it appears to do this incorrectly in Thumb mode. Adding a binutils task for this.

[Chris Coulson (chrisccoulson)]

Changing the start of sha256_block_data_order in sha256-armv4.S to avoid the use of the ADR pseudo-instruction like this:

global sha256_block_data_order
.type sha256_block_data_order,%function
sha256_block_data_order:
#ifdef __thumb2__
        sub r3,pc,#4 @ sha256_block_data_order
#else
        sub r3,pc,#8 @ sha256_block_data_order
#endif

... seems to work:

$ ../util/shlib_wrap.sh ./sha256t
Testing SHA-256 ... passed.
Testing SHA-224 ... passed.

[Chris Coulson (chrisccoulson)]

I also verified that with this workaround for the first instruction, the non-NEON path passes the test, by removing this block from sha256-armv4.S:

#if __ARM_MAX_ARCH__>=7 && !defined(__KERNEL__)
        ldr r12,.LOPENSSL_armcap
        ldr r12,[r3,r12] @ OPENSSL_armcap_P
        tst r12,#ARMV8_SHA256
        bne .LARMv8
        tst r12,#ARMV7_NEON
        bne .LNEON
#endif

[Chris Coulson (chrisccoulson)]

This is a deliberate change in binutils-2.29, which changes how the ADR instruction works with Thumb function symbols:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=52a86f843b6dee1de9977293da9786649b146b05

There are some changes in openssl which work around this:

https://git.openssl.org/?p=openssl.git;a=commit;h=11208dcfb9105e8afa37233185decefd45e89e17
https://git.openssl.org/?p=openssl.git;a=commit;h=b82acc3c1a7f304c9df31841753a0fa76b5b3cda

[Launchpad Janitor (janitor)]

This bug was fixed in the package openssl - 1.0.2g-1ubuntu13.2

---------------
openssl (1.0.2g-1ubuntu13.2) artful-security; urgency=medium

  * SECURITY UPDATE: Malformed X.509 IPAddressFamily could cause OOB read
    - debian/patches/CVE-2017-3735.patch: avoid out-of-bounds read in
      crypto/x509v3/v3_addr.c.
    - CVE-2017-3735
  * SECURITY UPDATE: bn_sqrx8x_internal carry bug on x86_64
    - debian/patches/CVE-2017-3736.patch: fix carry bug in
      bn_sqrx8x_internal in crypto/bn/asm/x86_64-mont5.pl.
    - CVE-2017-3736
  * debian/patches/fix_armhf_ftbfs.patch: fix build with gcc-7.2 on armhf.
    (LP: #1729850)

 -- Marc Deslauriers <email address hidden> Mon, 06 Nov 2017 07:56:00 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package openssl - 1.0.2g-1ubuntu14

---------------
openssl (1.0.2g-1ubuntu14) bionic; urgency=medium

  * SECURITY UPDATE: Malformed X.509 IPAddressFamily could cause OOB read
    - debian/patches/CVE-2017-3735.patch: avoid out-of-bounds read in
      crypto/x509v3/v3_addr.c.
    - CVE-2017-3735
  * SECURITY UPDATE: bn_sqrx8x_internal carry bug on x86_64
    - debian/patches/CVE-2017-3736.patch: fix carry bug in
      bn_sqrx8x_internal in crypto/bn/asm/x86_64-mont5.pl.
    - CVE-2017-3736
  * debian/patches/fix_armhf_ftbfs.patch: fix build with gcc-7.2 on armhf.
    (LP: #1729850)

 -- Marc Deslauriers <email address hidden> Mon, 06 Nov 2017 07:56:00 -0500