-
Notifications
You must be signed in to change notification settings - Fork 131
Description
Reproduction
- Build a hex with the partial flashing service enabled
- Connect via Bluetooth using e.g. nRF Connect on Android
- Send 0x0001 to the partial flashing service's characteristic to request a region
- 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));