smp_client_response_discover() matched a response to a pending command
on the sequence number and operation only. Sequence numbers are eight
bits, kept per client object and wrap, and all clients share one list
of pending commands, so a response could complete an unrelated command:
one from another client object, for another group or command id, or
one sent on another transport. On a device that is both an SMP server
and an SMP client, a peer on the server transport could complete a
client command, for example acknowledge an image upload offset that was
never written.
Only match a pending command that was sent on the transport the response
was received on and that carries the same group and command id. The
Zephyr SMP server and MCUboot serial recovery echo both in every
response, including error responses.
smp_client_single_response() takes the receiving transport as a new
first argument. Its only in-tree caller, smp_process_request_packet(),
passes streamer->smpt.
Tested with the mgmt.mcumgr.smp.client and mgmt.mcumgr.mcumgr.client
scenarios on qemu_x86.
Assisted-by: Claude:claude-opus-5.5
Signed-off-by: Jeff Welder <Jeff.Welder@ellenbytech.com>
The UDP SMP receive thread copied a received datagram into the SMP
net_buf with net_buf_add_mem() without checking that it fits. When
CONFIG_MCUMGR_TRANSPORT_NETBUF_SIZE is smaller than
CONFIG_MCUMGR_TRANSPORT_UDP_MTU a datagram larger than the net_buf
writes past it; without CONFIG_ASSERT or CONFIG_NET_BUF_HARDENING the
write is silent. The relation was only stated in help text, and that
text was wrong: it allowed the MTU to exceed the net_buf size.
Constrain it in two layers:
- Kconfig: give MCUMGR_TRANSPORT_UDP_MTU a range of
1..MCUMGR_TRANSPORT_NETBUF_SIZE, matching the SPI transport, and
default it to the net_buf size when that is below 1500. A build that
explicitly sets the MTU above the net_buf fails at configuration
time; a build that only lowered the net_buf keeps building with a
matching MTU. Fix the help text.
- Runtime: check net_buf_tailroom() before the copy and drop oversized
frames, matching the Bluetooth transport (smp_bt.c). len is bounded
by the UDP MTU on every receive path, so this cannot trigger while
those invariants hold, but it keeps the copy safe if the socket
layer or net_buf headroom assumptions change.
The default UDP build (net_buf 2048, MTU 1500) is unchanged. Document
the new constraint in the 4.5 migration guide.
Assisted-by: Claude:claude-opus-5-5
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: JP Hutchins <jp@intercreate.io>
Adds support for using Bluetooth Low Energy with transport
management, to allowing bridging (both incoming and outgoing)
connections to/from other MCUmgr transports
Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Signed-off-by: Jamie McCrae <spam@helper3000.net>
Refactors the code to make it more concise and easier to extend
Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Signed-off-by: Jamie McCrae <spam@helper3000.net>
Declare the mcumgr os_mgmt processor name for TriCore so the OS
management group reports the correct architecture.
Signed-off-by: Parthiban Nallathambi <parthiban@linumiz.com>
The shi_ite driver unmasks the SPI peripheral interrupt while
initializing the device at POST_KERNEL priority 40, but the rx
context is only assigned later, when ec_host_cmd_init() calls
shi_ite_backend_init() at priority 80.
If the host AP starts a transaction within that window, for instance
right after a warm jump from RO to RW, the interrupt fires before
data->rx_ctx is assigned. shi_ite_parse_header() then dereferences
the NULL pointer, which triggers a fatal store access fault
(mcause 7) when PMP null pointer detection is enabled.
Leave the interrupt masked and the state at SHI_STATE_DISABLED
during device init, then set both in shi_ite_backend_init() once
the buffers are known good.
Also check rx_ctx in the interrupt handler, so that the invariant
holds even if the interrupt is unmasked from another path. Reaching
the handler without a rx context is not expected, so log it, and
write clear the status before returning to avoid an interrupt storm.
Signed-off-by: Cheng Yueh <fenzhangs@gmail.com>
The slim ring buffer API is now header-only and always available and the
tree has already migrated from the _claim/_finish API, so the RING_BUFFER
symbol no longer gates any code.
Remove the now-redundant "select RING_BUFFER" and CONFIG_RING_BUFFER=y
entries across drivers, subsystems, samples and tests.
Signed-off-by: Måns Ansgariusson <mansgariusson@gmail.com>
The image management server reports 64-byte image digests when MCUboot
uses SHA-512. The client currently accepts only 32-byte state-response
digests and encodes that fixed length in state-write requests, preventing
it from listing or selecting images on such targets.
Support both 32-byte SHA-256 and 64-byte SHA-512 digests. Store the
received digest length and add a hash length argument to
img_mgmt_client_state_write().
Update the tests to cover both digest lengths.
Assisted-by: claude:opus-4.8
Signed-off-by: Vidar Berg <vidar.berg@nordicsemi.no>
The class API vtable and the descriptor pointer array are never written
after initialization, but were missing a const and so ended up in
.data. Add it.
No sample or test builds this backend, so the saving was not measured.
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Assisted-by: Claude:opus-5
Add the missing const (and static, where the symbol is file-local) to
the class API vtables and the per-instance arrays of descriptor
pointers, which nothing writes after initialization.
That requires usbd_class_api::get_desc() to return "const void *". The
stack walks the result as "struct usb_desc_header *const *", so the
descriptors stay writable for the bInterfaceNumber and
bEndpointAddress patching done at init. Classes outside subsys/usb
only get the new return type here.
With the previous commit, this saves 86 to 180 bytes of RAM per class
on nrf52840dk/nrf52840, with no flash increase.
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Assisted-by: Claude:opus-5
os_mgmt_echo() decodes the request into
struct zcbor_string data = { 0 };
with zcbor_map_decode_bulk(), which reports success when the map holds
none of the keys it was given: nothing matched is not an error. A
request whose payload is an empty map therefore reaches the encoder
with data still holding a NULL pointer and a length of zero, and the
reply is a successful echo of a string the peer never sent.
Handing that to zcbor_tstr_encode() is undefined behaviour rather than
a harmless no-op, because it reaches memmove(dst, NULL, 0) in
zcbor_encode.c. Ten bytes are enough, and MCUmgr parses them before
anything has authenticated the peer:
02 00 00 02 00 00 01 00 bf ff
Require the key the command is defined around. The reply becomes
rc = MGMT_ERR_EINVAL, which is what this handler already answers when
the payload does not decode, and a request that carries "d" is
unaffected.
The regression test sends that request through the dummy SMP transport
and requires the error response. It fails on an unmodified tree, where
the reply is 0x03 0x00 0x00 0x05 0x00 0x00 0x01 0x00 bf 61 72 60 ff,
an echo of the empty string.
Under the address and undefined behaviour sanitizers an unmodified tree
also reports
zcbor_encode.c:283:31: runtime error: null pointer passed as
argument 2, which is declared to never be null
for the same request, and does not with this commit applied. Seeing
that report needs -fsanitize=function switched off, because the cast in
ZCBOR_MAP_DECODE_KEY_DECODER trips the function-type check first on
every mcumgr command that matches a key; that is a separate matter and
is reported on its own.
Signed-off-by: Arkadiusz Grzelka <devitwise@gmail.com>
If building with TF-M and BL2, BOOTLOADER_MCUBOOT is `n`.
But, TF-M's BL2 is just a wrapper around mcuboot.
Hawkbit should be allowed if BOOTLOADER_MCUBOOT or TFM_BL2 are `y`.
Signed-off-by: Tyson Jones <tyson.jones@lvt.com>
Use controller/peripheral and SDO/SDI terminology in the MCUmgr SMP
SPI transport, following coding guideline rule A.2: switch to
SPI_OP_MODE_PERIPHERAL and the SPI_PERIPHERAL Kconfig dependency,
and update the transport test configuration and its bitbang overlay
to CONFIG_SPI_PERIPHERAL and the sdo-gpios/sdi-gpios properties.
Assisted-by: Claude:fable-5
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Use controller/peripheral and SDO/SDI terminology in the remaining
SPI-using drivers (audio, auxdisplay, bluetooth HCI, dac, eeprom,
ethernet, flash, fpga, gpio, haptics, hdlc_rcp_if, ieee802154,
input, led, led_strip, lora, mfd, misc, mux, rtc, sdhc, stepper,
video, watchdog, wifi) and the ec_host_cmd SPI STM32 backend,
following coding guideline rule A.2: switch to
SPI_OP_MODE_CONTROLLER/PERIPHERAL and SPI_SDO_OVERRUN_DT, rename
Zephyr-owned identifiers (the HCI SPI header buffers, the winc1500
bus wrapper transfer parameters), and update comments, log messages
and Kconfig help prose.
Vendor and out-of-scope identifiers keep their names: STM32
LL_SPI_MODE_SLAVE in the ec_host_cmd backend, the ATWINC1500 SDK I2C
SLAVE_ADDRESS definition, and the I2S codec clock-role variable in
tlv320aic26.
Assisted-by: Claude:fable-5
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
smp_rx_remove_invalid() used a k_fifo living on its stack as a scratch
queue while filtering buffers. Kernel object tracking
(CONFIG_OBJ_CORE_FIFO, object tracking) registers FIFOs passed to
k_fifo_init(), leaving the tracking lists referencing dead memory once
the function returns.
Use a plain sys_slist_t with the net_buf slist helpers instead; the
scratch queue never blocks and is fully drained before returning.
Signed-off-by: Pieter De Gendt <pieter.degendt@basalte.be>
Assisted-by: Claude:claude-fable-5
When there was a networking error in the s_http_start function,
the hawkbit state would change to terminate, and the s_http_end function
would run, freeing any k_malloc memory. But because there was no return
statement, the function would proceed to allocate memory, which would
not get freed because the state was already terminate which does not
have a cleanup exit function assigned.
Signed-off-by: Avary Higbee <avary.higbee@lvt.com>
Replace direct NVIC_SetPendingIRQ()/NVIC_ClearPendingIRQ() calls with
k_irq_set_pending()/k_irq_clear_pending(), dropping the dependency on
cmsis_core.h where nothing else needed it. These platforms are
plain-NVIC Cortex-M, so the pending-state capabilities are always
available where this code builds.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Update struct ec_host_cmd to unconditionally store the k_tid_t of the
thread executing host commands. When running with a dedicated thread,
store the k_tid_t returned by k_thread_create. When running via
ec_host_cmd_task, record k_current_get() upon entry.
This allows consumers to consistently query the host command thread via
the ec_host_cmd context regardless of the threading configuration.
Signed-off-by: Al Semjonovs <asemjonovs@google.com>
An MCUmgr image upload over the shell transport hard faults the device.
Not an error return, not a dropped frame - a fault in the shell_uart
thread, then a watchdog reset. It looks like a silent transport, because
by the time a host retries anything the board has rebooted and answers
normally.
mcumgr_serial_process_frag reads the two framing bytes of every line as
op = sys_be16_to_cpu(*(uint16_t *)frag);
frag points into a net_buf from the shell transport's pool, and 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. The cast is then an
unaligned 16-bit load.
ARMv7-M fixes those up in hardware, which is why this has gone unnoticed:
on a Cortex-M3, M4 or M33 the code is merely undefined behaviour that
happens to work. ARMv8-M Baseline does not. On a Cortex-M23 it is a
HardFault.
That is also why the failure looks selective rather than total. A request
that fits one framing line uses the first buffer in the pool, which is
aligned, and is answered: image state read works every time. A request
spanning two or more lines reaches the second buffer and faults. An image
upload cannot fit one line, because its first request carries image, len,
off and a 32-byte sha before any image data, so upload never works and
everything else does.
sys_get_be16() is the alignment-safe read and is what the rest of the
tree uses for exactly this. 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.
Found on a Microchip PIC32CM5112GC00100 (Cortex-M23). The faulting PC
resolves to serial_util.c:161, called from smp_shell_process at
smp_shell.c:213, in thread shell_uart.
Signed-off-by: Arkadiusz Grzelka <devitwise@gmail.com>
Remove CONFIG_MCUMGR_GRP_OS_INFO_HARDWARE_INFO_SHORT_HARDWARE_PLATFORM,
which was introduced already deprecated in Zephyr 4.3 as an opt-in back
to the pre-4.3 hardware platform output of the os_mgmt info command.
The info command now unconditionally reports CONFIG_BOARD_TARGET, which
includes the SoC and board variant. Builds that had opted back into the
board and board revision format see their SMP info response text change,
as announced by the 4.3 deprecation.
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Assisted-by: Claude:opus-5
Remove CONFIG_MCUBOOT_BOOTLOADER_MODE_SWAP_WITHOUT_SCRATCH, deprecated in
Zephyr 4.1 in favour of CONFIG_MCUBOOT_BOOTLOADER_MODE_SWAP_USING_MOVE,
which selects the same MCUBOOT_BOOTLOADER_MODE_HAS_NO_DOWNGRADE and maps
to the same MCUBOOT_MODE_SWAP_USING_MOVE bootloader info value.
The MCUmgr img_mgmt and os_mgmt users only ever OR'd the deprecated
symbol with its replacement, so dropping the disjunct is behaviour
preserving. The sysbuild counterpart was already removed.
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Assisted-by: Claude:opus-5
Adds support for bridging using this transport
Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Signed-off-by: Jamie McCrae <spam@helper3000.net>
Adds support for bridging using this transport
Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Signed-off-by: Jamie McCrae <spam@helper3000.net>
Adds a new experimental group which supports transport bridging
Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Signed-off-by: Jamie McCrae <spam@helper3000.net>
Changes the priority from positive to negative to make it a
cooperative thread, this prevents issues when using ztest which make
the transport function stop running when a semaphore is given, which
eventually leads to a lack of network buffers
Signed-off-by: Jamie McCrae <spam@helper3000.net>
Adds a function that can be enabled to show how many MCUmgr buffers
are available. Useful for tests, and was used to notice an issue
with ztest and default MCUmgr transport workqueue priority
Signed-off-by: Jamie McCrae <spam@helper3000.net>
cp_decode_response() copies REPLY_KEYPPAD and REPLY_RAW payloads into
64 byte arrays of a stack allocated osdp_event, checking only that the
declared length matches the frame. Bound it by OSDP_EVENT_MAX_DATALEN.
Signed-off-by: Flavio Ceolin <flavio@hubble.com>
sha256sum is an optional JSON string field on a zero-initialised struct, so
a server response omitting it left the pointer NULL and strlen()
dereferenced it before the length check. Both metadata branches.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Add an SPI SMP transport for mcumgr where the device operates as an
SPI peripheral and exchanges raw SMP packets with the SPI controller,
with no additional framing layer. A single data-ready GPIO signals
the controller when a response is ready to be read, avoiding a
resident polling thread. Zero-byte packets are handled via
smp_reassembly_drop() to clear a partial reassembly buffer.
Includes the Kconfig, the zephyr,smp-spi devicetree binding, and the
SMP_SPI transport enum entry.
Signed-off-by: Sanjay Varghese <Varghesemela@gmail.com>
Prevent race condition where a secondary SPI request header
could overwrite in_msg while host command processing is active.
1. Add state check to ignore CS callbacks and RVLI interrupts when not
in SHI_STATE_READY_TO_RECV / SHI_STATE_RECEIVING.
2. Read header into a local buffer for length and version validation
before copying into in_msg.
Signed-off-by: Al Semjonovs <asemjonovs@google.com>
Renames functions to have raw in the names, this helps when
debugging with both raw and raw dummy transports enabled, as
without the change the debugger will show the names as the same
Signed-off-by: Jamie McCrae <spam@helper3000.net>
Apply the default pinctrl state during PM_DEVICE_ACTION_RESUME to restore
normal SHI operation.
Apply the sleep pinctrl state during PM_DEVICE_ACTION_SUSPEND to disable
the SHI interface pins and prevent leakage.
This lets board/shim code control the pin state purely via
pm_device_runtime_get()/put().
Signed-off-by: Tim Lin <tim2.lin@ite.corp-partner.google.com>
settings_mgmt_read()/_write()/_delete() each allocate key_name (and
data, for read) from the heap when CONFIG_MCUMGR_GRP_SETTINGS_
BUFFER_TYPE_HEAP is enabled, then optionally call an application
access hook via mgmt_callback_notify(). When that hook returns
MGMT_CB_ERROR_RC, the handler does 'return ret_rc;' directly,
bypassing the 'end:' label where key_name/data would otherwise be
freed. Every other status/return path in these functions does go
through 'end:' (or frees explicitly) and is not affected.
Free the heap-allocated buffers before returning on this specific
path, matching what the 'end:' label already does for the other
paths in the same functions.
Fixes#112782
Signed-off-by: yi chen <94xhn1@gmail.com>
memcpy(metadata_copy, metadata, strlen(metadata)) omits the trailing
NUL, so the later strlen(metadata_copy) in the fallback parse path
can read past the allocation and feed an oversized length into
json_obj_parse().
Signed-off-by: Flavio Ceolin <flavio@hubblenetwork.com>
cast uint8_t to uint32_t before << 24 to avoid signed UB.
By undefined behavior sanitizer:
runtime error: left shift of 128 by 24 places cannot be
represented in type 'int'
Signed-off-by: Flavio Ceolin <flavio@hubblenetwork.com>
ctx->usb_rx_buf is a persistent member of the device context struct.
On error paths where the enqueue fails, the buffer was unreferenced
but the pointer was left dangling. Use net_buf_drop() to both
unreference and clear the pointer in one step.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
net_buf_unref(buf);
buf = NULL;
with net_buf_drop(&buf) which performs both operations
atomically in a single step.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
