artful openssl FTBFS on armhf

Bug #1729850 reported by Marc Deslauriers on 2017-11-03
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
binutils (Ubuntu)
Status tracked in Bionic
Artful
Undecided
Unassigned
Bionic
Undecided
Unassigned
gcc-7 (Ubuntu)
Status tracked in Bionic
Artful
Undecided
Unassigned
Bionic
Undecided
Unassigned
openssl (Ubuntu)
Status tracked in Bionic
Artful
Undecided
Marc Deslauriers
Bionic
Undecided
Marc Deslauriers

Bug Description

openssl FTBFS on artful armhf with the following:

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

CVE References

Marc Deslauriers (mdeslaur) wrote :

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) wrote :

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) wrote :

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) wrote :
Download full text (3.2 KiB)

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...

Read more...

Chris Coulson (chrisccoulson) wrote :

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) wrote :

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) wrote :

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

Changed in gcc-7 (Ubuntu):
status: New → Invalid
Changed in binutils (Ubuntu):
status: New → Invalid
Changed in gcc-7 (Ubuntu Artful):
status: New → Invalid
Changed in binutils (Ubuntu Artful):
status: New → Invalid
Changed in openssl (Ubuntu Artful):
status: New → Confirmed
Changed in openssl (Ubuntu Bionic):
status: New → In Progress
Changed in openssl (Ubuntu Artful):
status: Confirmed → In Progress
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in openssl (Ubuntu Bionic):
assignee: nobody → Marc Deslauriers (mdeslaur)
Launchpad Janitor (janitor) wrote :

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

Changed in openssl (Ubuntu Artful):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers