Image upload Write missing len parameter

[vChavezB]

I am testing the smpmgr and found a possible bug (?) when uploading an image to mcuboot over zephyr.

If the ImageUploadWrite packet does not include the len parameter, when the bootloader decodes the len parameter it gets the maximum uint32_t number. This in turn makes mcuboot return MGMT_ERR_EINVAL and makes the upload from smpmgr fail as the offset field is not sent since rc == 3.

Mcuboot fails because the first chunk uses the len field to check if the length of the image fits in the partition (here).

If I add the len field here, then mcuboot complies and works correctly. Something such as

            response = await self.request(
                self._maximize_packet(ImageUploadWrite(off=response.off, data=b''), image, len= len(image))
            )

I tested this with the latest mucboot repo from zephyr. In specific this commit.

Smpmgr options

upgrade zephyr.signed.hex

[vChavezB]

It seems that the SMP docs (latest version from zephyr) mentions that for off = 0, len should be used.

https:/zephyrproject-rtos/zephyr/blob/8e13268439b2e9c6666c26b43b31462354b883ce/doc/services/device_mgmt/smp_groups/smp_group_1.rst?plain=1#L296

[JPHutchins]

It seems that the SMP docs (latest version from zephyr) mentions that for off = 0, len should be used.

This is correct, and this is what the provided upload routine does. It makes one initial request that contains the information that is only used by the first request: len, sha, and upgrade:

smpclient/smpclient/__init__.py

Lines 57 to 69 in 9279cd7

	response = await self.request(
	self._maximize_packet(
	ImageUploadWrite( # type: ignore
	off=0,
	data=b'',
	image=slot,
	len=len(image),
	sha=sha256(image).digest(),
	upgrade=upgrade,
	),
	image,
	)
	)

Subsequent requests do not include the len, sha, or upgrade fields.

If I add the len field here, then mcuboot complies and works correctly. Something such as

I suspect that the root cause of the issue is the MCUBoot SMP MTU. smpmgr and the other tools will default to 4096 whereas your version of MCUBoot may be lower. Here is some info from Kconfig.serial_recovery:

config BOOT_MAX_LINE_INPUT_LEN
	int "Maximum input line length"
	default 128
	help
	  Maximum length of input serial port buffer (SMP serial transport uses
	  fragments of 128-bytes, this should not need to be changed unless a
	  different value is used for the transport).

config BOOT_SERIAL_MAX_RECEIVE_SIZE
	int "Maximum command line length"
	default 1024
	help
	  Maximum length of received commands via the serial port (this should
	  be equal to the maximum line length, BOOT_MAX_LINE_INPUT_LEN times
	  by the number of receive buffers, BOOT_LINE_BUFS to allow for
	  optimal data transfer speeds).

I've been using

CONFIG_BOOT_MAX_LINE_INPUT_LEN=8192
CONFIG_BOOT_SERIAL_MAX_RECEIVE_SIZE=4096

And get ~20-55KBps depending on the hardware.

From rereading the Kconfig info I see that my config is wrong. Seems that it should be CONFIG_BOOT_SERIAL_MAX_RECEIVE_SIZE = CONFIG_BOOT_LINE_BUFS * CONFIG_BOOT_MAX_LINE_INPUT_LEN

CONFIG_BOOT_MAX_LINE_INPUT_LEN is indeed statically allocated in serial_adapter.c, and that's the value that needs to be GTE the --mtu provided by smpmgr (or smpclient), default to 4096.

Looks like CONFIG_BOOT_SERIAL_MAX_RECEIVE_SIZE * 2 will be statically allocated in boot_serial.c.

tldr; try with --mtu 128 or even lower, then experiment with updating your MCUBoot build. Here is a ticket where I first observed strange behavior if the first packet if it doesn't fit in MTU: intercreate/smpmgr#8

Cheers,
JP

[JPHutchins]

Unrelated, this should be investigated more. Why a default of 8 buffers? This only makes sense to me if the MCUBoot serial driver was smart enough to be doing async RX on a DMA while it's writing the new image to flash - it is not doing that. Everything should be synchronous:

1. receive bytes
2. if a complete SMP packet has been received, continue, otherwise wait for more bytes
3. parse the packet, write to flash (maybe), and return error or new offset

The SMP client would not be sending the next upload request until it has received the next offset. I don't see a reason for more than one buffer, yet I see 10 (8 in serial_adapter.c and 2 in boot_serial.c) in the default implementation.

[vChavezB]

I am actually using mtu of 128. I am prototyping with CAN as a transport layer getting around 4 kb/s with 1 MHz , 8 bytes DLC.

I will try to increase the buffer size. Problem is my mcu has around 16 kb sram.

[JPHutchins]

I am actually using mtu of 128. I am prototyping with CAN as a transport layer getting around 4 kb/s with 1 MHz , 8 bytes DLC.

CAN, very cool!

This explains the problem and it is the same as what I saw in the shell transport. The fix is to remove the hash from the first request and use —MTU 128. Or to increase your MTU in FW to ~256. I don’t know of an SMP server that uses the hash - they should, LMAO.

How are you building MCUBoot and what options are available to you? I am only familiar with Zephyrs SMP server implementation but can look at others.

I am very curious if you're using Zephyr with 16K of RAM.

[JPHutchins]

• here img_size_tmp is set to SIZE_MAX 🙄 : https:/zephyrproject-rtos/mcuboot/blob/d4394c2f9b76e0a7b758441cea3a8ceb896f66c8/boot/boot_serial/src/boot_serial.c#L642
• here the CBOR decode ignores the SHA: https:/zephyrproject-rtos/mcuboot/blob/d4394c2f9b76e0a7b758441cea3a8ceb896f66c8/boot/boot_serial/src/boot_serial.c#L662-L667
• img_size_tmp is not valid: https:/zephyrproject-rtos/mcuboot/blob/d4394c2f9b76e0a7b758441cea3a8ceb896f66c8/boot/boot_serial/src/boot_serial.c#L735-L747
• or img_size_tmp is assigned to the mutable static variable img_size: https:/zephyrproject-rtos/mcuboot/blob/d4394c2f9b76e0a7b758441cea3a8ceb896f66c8/boot/boot_serial/src/boot_serial.c#L761

Looking at serial_adapter.c next.

It's sorta complicated and I can see why multiple buffers may be required for a poorly behaved client. Seems like changing the CONFIG_BOOT_MAX_LINE_INPUT_LEN to be larger might get you by. I don't see the need for 8 buffers of it, try 2 maybe.

https:/zephyrproject-rtos/mcuboot/blob/8dab2b975e14e00564eb62383448f70efd01f324/boot/zephyr/serial_adapter.c

[vChavezB]

I am building MCUBoot for Zephyr with sysbuild. I am actually using the default options provided by MCUboot and using a single application slot (CONFIG_SINGLE_APPLICATION_SLOT=y).

You can see my forked version here. I basically copied the serial transport adapter from zephyr and use the can api instead.

I am very curious if you're using Zephyr with 16K of RAM.

The MCU has 32 K but they are not contigous. The SoC definition provided by NXP only uses half of it. The other half requires an extra linker file to be used for buffers,etc. So far my application and bootloader work with 16 K but perhaps I will need to enable the second half.

[JPHutchins]

The MCU has 32 K but they are not contigous

Zephyr with only 32K of RAM is wild 🔥

I've used single application slot and would like to make you aware of this: mcu-tools/mcuboot#1931

[vChavezB]

Thanks, this is something good to be aware of! For my use case I have an external GPIO to get into the bootloader.

I will try the buffer recommendation and see if that makes mcuboot happy.

Btw, thanks a lot for the project! This has allowed me to prototype a CAN MCuboot loader really fast.

[JPHutchins]

You can see my forked version here

This is awesome! I'd love to see it upstreamed to MCUBoot though I understand that's a lot of time. Since you have control over the defaults, I recommend proceeding to the larger buffer sizes. If the --mtu 256 combined with updating the defaults resolves the issue, then I am confident that the fix for smpclient/smpmgr is to not include the SHA if the MTU is under a certain size.

Last thing - I'm unclear if you tried running smpmgr --mtu 128 upgrade zephyr.signed.hex? It'd be nice if that works without more invasive changes.

And keep me posted if smpclient needs a CAN transport. Sounds like you're using some adapter for now.

[vChavezB]

The buffer increase did the trick! Changed it to 512 max line input, 2 buffers and now I am getting around 15 Kb/s.