Describe the bug
mcumgr_serial_process_frag() reads the two framing bytes of every MCUmgr line through a cast:
op = sys_be16_to_cpu(*(uint16_t *)frag);
frag points into a net_buf from the shell transport's pool. That pool is defined in subsys/shell/backends/shell_uart.c with a data size of SMP_SHELL_RX_BUF_SIZE, which is 127. NET_BUF_POOL_FIXED_DEFINE aligns the array but strides its rows by the data size, so with an odd stride every other buffer in the pool starts on an odd address, and the read is an unaligned 16-bit load.
ARMv7-M fixes such a load up in hardware, so on Cortex-M3, M4 and M33 this is undefined behaviour that happens to work. ARMv8-M Baseline does not. On a Cortex-M23 it is a HardFault.
To Reproduce
- Build an application for an ARMv8-M Baseline target (Cortex-M23) with the MCUmgr shell transport:
CONFIG_MCUMGR_TRANSPORT_SHELL=y.
- Run
mcumgr image list over the console. It answers.
- Run
mcumgr image upload. The device faults in the shell_uart thread and a watchdog reset follows.
Expected behavior
The upload completes.
Impact
DFU over the MCUmgr shell transport is unusable on every ARMv8-M Baseline part.
The failure looks selective rather than total, which is what makes it hard to recognise: a request that fits one framing line uses the first buffer in the pool, which is aligned, and is answered, so image state read works every time. A request spanning two or more lines reaches the second buffer and faults, and an upload cannot fit one line because its first request carries image, len, off and a 32-byte sha before any image data.
Environment
Found on a Microchip PIC32CM5112GC00100 (Cortex-M23) with the Zephyr SDK, on main. The faulting PC resolves to serial_util.c:161, called from smp_shell_process at smp_shell.c:213, in thread shell_uart.
Additional context
Fixed on main by #116813, which uses the alignment-safe sys_get_be16(). This issue exists so that the backports to the release branches have something to reference, as scripts/release/list_backports.py requires.
The fix does not depend on the buffer size, which is the other half of the story: SMP_SHELL_RX_BUF_SIZE is a hardcoded macro rather than a Kconfig symbol, so no application can round the stride up to avoid this.
Affected: v3.7-branch, v4.3-branch, v4.4-branch.
Describe the bug
mcumgr_serial_process_frag()reads the two framing bytes of every MCUmgr line through a cast:fragpoints into a net_buf from the shell transport's pool. That pool is defined insubsys/shell/backends/shell_uart.cwith a data size ofSMP_SHELL_RX_BUF_SIZE, which is 127.NET_BUF_POOL_FIXED_DEFINEaligns the array but strides its rows by the data size, so with an odd stride every other buffer in the pool starts on an odd address, and the read is an unaligned 16-bit load.ARMv7-M fixes such a load up in hardware, so on Cortex-M3, M4 and M33 this is undefined behaviour that happens to work. ARMv8-M Baseline does not. On a Cortex-M23 it is a HardFault.
To Reproduce
CONFIG_MCUMGR_TRANSPORT_SHELL=y.mcumgr image listover the console. It answers.mcumgr image upload. The device faults in theshell_uartthread and a watchdog reset follows.Expected behavior
The upload completes.
Impact
DFU over the MCUmgr shell transport is unusable on every ARMv8-M Baseline part.
The failure looks selective rather than total, which is what makes it hard to recognise: a request that fits one framing line uses the first buffer in the pool, which is aligned, and is answered, so
image state readworks every time. A request spanning two or more lines reaches the second buffer and faults, and an upload cannot fit one line because its first request carriesimage,len,offand a 32-byteshabefore any image data.Environment
Found on a Microchip PIC32CM5112GC00100 (Cortex-M23) with the Zephyr SDK, on
main. The faulting PC resolves toserial_util.c:161, called fromsmp_shell_processatsmp_shell.c:213, in threadshell_uart.Additional context
Fixed on
mainby #116813, which uses the alignment-safesys_get_be16(). This issue exists so that the backports to the release branches have something to reference, asscripts/release/list_backports.pyrequires.The fix does not depend on the buffer size, which is the other half of the story:
SMP_SHELL_RX_BUF_SIZEis a hardcoded macro rather than a Kconfig symbol, so no application can round the stride up to avoid this.Affected: v3.7-branch, v4.3-branch, v4.4-branch.