Skip to content

Crash due to undefined behaviour in MicrobitMemoryMap's Region constructor #499

@microbit-matt-hillsdon

Description

@microbit-matt-hillsdon

Reproduction

  1. Build a hex with the partial flashing service enabled
  2. Connect via Bluetooth using e.g. nRF Connect on Android
  3. Send 0x0001 to the partial flashing service's characteristic to request a region
  4. Observe Bluetooth disconnect and LED pattern indicating crash

Analysis

The crash is in the Region constructor as called by the MicrobitMemoryMap constructor.

We call e.g. pushRegion(Region(0x00, 0x00, 0x18000, 0x00)); in MemoryMap (multiple similar calls).

That last 0x00 is NULL being passed as uint8_t hash[8]. The constructor calls memcpy( this->hash, hash, 8 ); and memcpy with a null source is UB.

If you disassemble you can see it has compiled to udf #0xff as the compiler infers UB on this path and replaces it with a trap:

Dump of assembler code from 0x24a90 to 0x24ab0:
   0x00024a90 <_ZN17MicroBitMemoryMapC2Ev+28>:  adds    r0, #12
   0x00024a92 <_ZN17MicroBitMemoryMapC2Ev+30>:  adds    r6, #20
   0x00024a94 <_ZN17MicroBitMemoryMapC2Ev+32>:  bl      0x261fc <memset>
   0x00024a98 <_ZN17MicroBitMemoryMapC2Ev+36>:  cmp     r6, r5
   0x00024a9a <_ZN17MicroBitMemoryMapC2Ev+38>:  bne.n   0x24a82 <_ZN17MicroBitMemoryMapC2Ev+14>
   0x00024a9c <_ZN17MicroBitMemoryMapC2Ev+40>:  add     r3, sp, #16
   0x00024a9e <_ZN17MicroBitMemoryMapC2Ev+42>:  ldmia   r4!, {r1, r2}
   0x00024aa0 <_ZN17MicroBitMemoryMapC2Ev+44>:  stmia   r3!, {r1, r2}
   0x00024aa2 <_ZN17MicroBitMemoryMapC2Ev+46>:  udf     #255    @ 0xff
   0x00024aa4 <_ZN3BLE18initImplementationE26FunctionPointerWithContextIPNS_37InitializationCompleteCallbackContextEE+0>:       movs    r2, r0
   0x00024aa6 <_ZN3BLE18initImplementationE26FunctionPointerWithContextIPNS_37InitializationCompleteCallbackContextEE+2>:       movs    r3, r1
   0x00024aa8 <_ZN3BLE18initImplementationE26FunctionPointerWithContextIPNS_37InitializationCompleteCallbackContextEE+4>:       push    {r4, r5, r6, r7, lr}
   0x00024aaa <_ZN3BLE18initImplementationE26FunctionPointerWithContextIPNS_37InitializationCompleteCallbackContextEE+6>:       ldr     r2, [r2, #0]
   0x00024aac <_ZN3BLE18initImplementationE26FunctionPointerWithContextIPNS_37InitializationCompleteCallbackContextEE+8>:       sub     sp, #36 @ 0x24
   0x00024aae <_ZN3BLE18initImplementationE26FunctionPointerWithContextIPNS_37InitializationCompleteCallbackContextEE+10>:      str     r2, [sp, #4]
End of assembler dump.

I built my hex using https://github.com/carlosperate/docker-microbit-toolchain/ which uses gcc 10. I assume MakeCode must be using a version and/or flags that means this doesn't trip them up as it works as expected in a MakeCode hex.

CODAL has the same bug though it's harder to trigger as the regions are only initialized when the MakeCode/Python magic is found. Again it works in practice in MakeCode hex files, though a toolchain update could break it.

Possible fix

Pass a proper empty hash or perhaps introduce a three-arg Region constructor.

uint8_t zeroHash[8] = {0};
pushRegion(Region(0x00, 0x00, 0x18000, zeroHash));

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions