Skip to content

Commit

Permalink
boot_serial: fix image number handle in image upload request
Browse files Browse the repository at this point in the history
According to the SMP protocol documentation [1], 'image number' value
in 'image upload request' is optional and can be included only in packet
with 'off' (data offset) set to '0' (first packet in upload request).

In one of recent changes (commit 'cb07e888691d'), initialization of the
'img_num' variable was removed which, in extreme case (no image number
provided in upload request), results in use of its uninitialized value
in flash_area_open() call which then might lead to request abort.

This fixes above regression and also makes MCUboot implementation of the
'image upload request' aligned with Zephyr documentation of the protocol
by considering image number only from first (off == 0) 'image upload
request' SMP packet. In addition, 'image number' value is set to '0' if
the request doesn't provide this field.

[1] docs.zephyrproject.org/latest/services/device_mgmt/smp_groups/smp_group_1.html

Fixes: cb07e88 ("boot_serial: Replace cbor auto-generated code with zcbor functions")
Signed-off-by: Piotr Dymacz <pepe2k@gmail.com>
  • Loading branch information
pepe2k authored and d3zd3z committed Jul 5, 2023
1 parent f2cb550 commit 6a8746d
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions boot/boot_serial/src/boot_serial.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <ctype.h>
#include <stdio.h>
#include <errno.h>
#include <limits.h>

#include "sysflash/sysflash.h"

Expand Down Expand Up @@ -617,7 +618,8 @@ bs_upload(char *buf, int len)
size_t img_chunk_off = SIZE_MAX; /* Offset of image chunk within image */
uint8_t rem_bytes; /* Reminder bytes after aligning chunk write to
* to flash alignment */
uint32_t img_num;
uint32_t img_num_tmp = UINT_MAX; /* Temp variable for image number */
static uint32_t img_num = 0;
size_t img_size_tmp = SIZE_MAX; /* Temp variable for image size */
const struct flash_area *fap = NULL;
int rc;
Expand All @@ -639,7 +641,7 @@ bs_upload(char *buf, int len)
zcbor_new_state(zsd, sizeof(zsd) / sizeof(zcbor_state_t), (uint8_t *)buf, len, 1);

struct zcbor_map_decode_key_val image_upload_decode[] = {
ZCBOR_MAP_DECODE_KEY_DECODER("image", zcbor_uint32_decode, &img_num),
ZCBOR_MAP_DECODE_KEY_DECODER("image", zcbor_uint32_decode, &img_num_tmp),
ZCBOR_MAP_DECODE_KEY_DECODER("data", zcbor_bstr_decode, &img_chunk_data),
ZCBOR_MAP_DECODE_KEY_DECODER("len", zcbor_size_decode, &img_size_tmp),
ZCBOR_MAP_DECODE_KEY_DECODER("off", zcbor_size_decode, &img_chunk_off),
Expand Down Expand Up @@ -672,6 +674,15 @@ bs_upload(char *buf, int len)
goto out_invalid_data;
}

/* Use image number only from packet with offset == 0. */
if (img_chunk_off == 0) {
if (img_num_tmp != UINT_MAX) {
img_num = img_num_tmp;
} else {
img_num = 0;
}
}

#if !defined(MCUBOOT_SERIAL_DIRECT_IMAGE_UPLOAD)
rc = flash_area_open(flash_area_id_from_multi_image_slot(img_num, 0), &fap);
#else
Expand Down

0 comments on commit 6a8746d

Please sign in to comment.