Mcumgr img_mgmt_impl_upload_inspect() can cause unaligned memory access hard fault.

[lgehreke]

Describe the bug

A firmware update using mcumgr with serial backend can result in a hard fault because of a unaligned
memory access since the new zcbor library is used.

During a firmware update packets are handled in zephyr_smp_handle_reqs(). The data is pulled from a netbuffer structure. This is passed through some layers to img_mgmt_upload(), which parses
the payload with zcbor_map_decode_bulk(). In the zcbor_map_decode_key_val the output buffers are set as pointers to members in the img_mgmt_upload_req structure. The problem is the img_data member in the request structure. This is now a zcbor_string: struct zcbor_string { const uint8_t *value; size_t len; };. The value can be set to a arbitrary address inside the netbuffer (zcbor_decode.c:value_extract() in the cbor library. See logs for gdb output). This becomes a problem during processing in img_mgmt_impl_upload_inspect(). This value is casted to a image_header here. Since this address can be unaligned, this will result in a hard fault in the next line if the processor does not support unaligned memory access!

Tested on:

• rpi_pico: https://github.com/yonsch/zephyr/tree/pico_flash

To Reproduce
Build the mcumgr smp_svr sample for a mcu that does not support unaligned memory access and
update with mcumgr image upload

Expected behavior
Mcumgr should perform a upload of the new firmware image.

Impact
Product cannot be updated.

Logs and console output

The following gdb output shows how zcbor_map_decode_bulk() changes
the req structure.

(gdb) p req
$2 = {image = 0, off = 4294967295, size = 4294967295, img_data = {value = 0x0, len = 0}, data_sha = {value = 0x0,
    len = 0}, upgrade = false}
(gdb) n
target halted due to debug-request, current mode: Thread
xPSR: 0x01000000 pc: 0x00000138 msp: 0x20041f00
370     ok = zcbor_map_decode_bulk(zsd, image_upload_decode,
=> 0x10012062 <img_mgmt_upload+130>:    30 00   movs    r0, r6
   0x10012064 <img_mgmt_upload+132>:    0d ab   add r3, sp, #52 ; 0x34
   0x10012066 <img_mgmt_upload+134>:    24 a9   add r1, sp, #144    ; 0x90
   0x10012068 <img_mgmt_upload+136>:    06 f0 0d fe bl  0x10018c86 <zcbor_map_decode_bulk>
(gdb) n
target halted due to debug-request, current mode: Thread
xPSR: 0x01000000 pc: 0x00000138 msp: 0x20041f00
375     if (!ok) {
=> 0x1001206c <img_mgmt_upload+140>:    00 28   cmp r0, #0
   0x1001206e <img_mgmt_upload+142>:    07 d1   bne.n   0x10012080 <img_mgmt_upload+160>
(gdb) p req
$3 = {image = 0, off = 0, size = 987136, img_data = {
    value = 0x20005f1b <net_buf_data_pkt_pool+19> "=\270", <incomplete sequence \363\226>, len = 295}, data_sha = {
    value = 0x2000605d <net_buf_data_pkt_pool+341> "\020\267=\005Wy\263\066@\314yW\370\032\255!\026\245\331\n\263\211\302⾺<\234'\275ϟG\024", len = 32}, upgrade = false}

Note that img_data.value is unaligend! During execution of img_mgmt_impl_upload_inspect() this leads to a hard fault:

531         if (hdr->ih_magic != IMAGE_MAGIC) {
=> 0x10011cf8 <img_mgmt_impl_upload_inspect+52>:    32 68   ldr r2, [r6, #0]
   0x10011cfa <img_mgmt_impl_upload_inspect+54>:    26 4b   ldr r3, [pc, #152]  ; (0x10011d94 <img_mgmt_impl_upload_inspect+208>)
   0x10011cfc <img_mgmt_impl_upload_inspect+56>:    9a 42   cmp r2, r3
   0x10011cfe <img_mgmt_impl_upload_inspect+58>:    ec d1   bne.n   0x10011cda <img_mgmt_impl_upload_inspect+22>
(gdb) n
z_arm_hard_fault () at /data/git/zephyr-workspace/zephyr/arch/arm/core/aarch32/cortex_m/fault_s.S:80
80      mrs r0, MSP
=> 0x100116d4 <z_arm_hard_fault+0>: ef f3 08 80 mrs r0, MSP

Environment (please complete the following information):

• OS: Linux
• Toolchain Zephyr SDK 0.14

Additional context
rpi_pico.overlay

/ {
	chosen {
		zephyr,code-partition = &slot0_partition;
		zephyr,settings-partition = &storage_partition;
		zephyr,uart-mcumgr = &uart0;
	};
};

&flash0 {
	erase-block-size = <4096>;

	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		/* Reserved memory for the second stage bootloader */
		second_stage_bootloader: partition@0 {
			label = "second_stage_bootloader";
			reg = <0x00000000 0x100>;
			read-only;
		};

		/* Partition for mcuboot */
		boot_partition: partition@100 {
			label = "mcuboot";
			reg = <0x100 0xcf00>;
		};

		/*
		 * Usable flash. Starts at 0xD000, after the bootloader(s). 
		 * Both code partitions have 1MB. We need 0x7000 bytes for
		 * the storage partition, 0x100 bytes for the second stage bootloader,
		 * 0xcf00 bytes for mcuboot and 0xA000 bytes for the scratch partition.
		 * This sums up to 0x1E000 bytes so we need to subtract 0xF000 bytes from
		 * both code partitions.
		 */
		slot0_partition: partition@d000 {
			label = "image-0";
			reg = <0xD000 (DT_SIZE_M(1) - 0xF000)>;
		};

		slot1_partition: partition@fe000 {
			label = "image-1";
			reg = <0xFE000 (DT_SIZE_M(1) - 0xF000)>;
		};

		scratch_partition: partition@1ef000 {
			label = "image-scratch";
			reg = <0x1EF000 0xA000>;
		};

		storage_partition: partition@1f9000 {
			label = "storage";
			reg = <0x1F9000 0x00007000>;
		};
	};
};

[nordicjm]

@lgehreke Give #49361 a try and see if it resolves the issue