[PATCH] tools: fwumdata: fix redundant metadata buffer overlap

read_metadata() allocates a single buffer holding both metadata copies
(calloc(2, mdata_size)) but accesses each copy through &mdata[id], where
mdata is a "struct fwu_mdata *". That stride is sizeof(struct fwu_mdata),
i.e. the 16-byte common header only, not mdata_size, so reading the
second copy overwrites the body of the first and the two copies overlap.

When both copies are identical the overlap is harmless, but as soon as
they differ (e.g. after an interrupted update, the exact case the tool is
meant to recover) the primary is corrupted: for V2 the metadata_size
check then reads garbage and the tool aborts with a bogus size mismatch,
for V1 it silently operates on a mixed view and can write it back to both
copies.

Index the copies by byte offset (mdata_size) through a small mdata_copy()
helper instead of struct-typed pointer arithmetic. Both copies are
guaranteed to share the same size (checked in parse_config()).

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Patrice Chotard <[email protected]>
---
Way to reproduce/check the issue is fixed :

cd /tmp
cat > gen_mdata.py <<'PY'
import struct, zlib
def build(active, prev, bank_state, img_guid_byte):
    version=2; num_banks=2; num_images=1
    sz_mdata=16; sz_ext=16; sz_desc=8; sz_img_entry=32; sz_bank_info=24
    per_image=sz_img_entry+sz_bank_info*num_banks
    total=sz_mdata+sz_ext+sz_desc+per_image*num_images
    ext=struct.pack('<IHH4BI', total, sz_mdata+sz_ext, 0, *bank_state, 0)
    img_entry_size=sz_img_entry+sz_bank_info*num_banks
    desc=struct.pack('<BBHHH', num_banks, 0, num_images, img_entry_size, sz_bank_info)
    body=bytes([0x11]*16)+bytes([0x22]*16)
    for b in range(num_banks):
        body+=bytes([img_guid_byte+b]*16)+struct.pack('<II',1,0)
    payload=struct.pack('<I',version)+struct.pack('<II',active,prev)+ext+desc+body
    crc=zlib.crc32(payload)&0xffffffff
    return struct.pack('<I',crc)+payload
open('mdA.bin','wb').write(build(0,1,[0xFC,0xFE,0,0],0x30))  # active=0
open('mdB.bin','wb').write(build(1,0,[0xFE,0xFF,0,0],0x50))  # active=1
PY
python3 gen_mdata.py; cp mdA.bin mdA2.bin
printf '/tmp/mdA.bin 0x0 0x78 0x78\n/tmp/mdB.bin 0x0 0x78 0x78\n' > cfg_diff.config
printf '/tmp/mdA.bin 0x0 0x78 0x78\n/tmp/mdA2.bin 0x0 0x78 0x78\n' > cfg_same.config
printf '/tmp/mdA.bin 0x0 0x78 0x78\n' > cfg_single.config

echo "===== FIXED: differing redundant copies (previously: 'Cannot read metadata', rc=234) ====="
/tmp/fwumdata_fixed -c cfg_diff.config 2>&1 | head -8; echo "rc=${PIPESTATUS[0]:-$?}"
echo; echo "===== FIXED: identical redundant copies (regression check) ====="
/tmp/fwumdata_fixed -c cfg_same.config 2>&1 | head -6; echo "rc=$?"
echo; echo "===== FIXED: single copy (regression check) ====="
/tmp/fwumdata_fixed -c cfg_single.config 2>&1 | head -6; echo "rc=$?"
rm -f /tmp/gen_mdata.py /tmp/md*.bin /tmp/cfg_*.config /tmp/fwumdata_fixed

    ===== FIXED: differing redundant copies (previously: 'Cannot read metadata', rc=234) =====
    Metadatas valid but not equal, use first one as default
    FWU Metadata:
            Version:            2
            Active Index:       0
            Previous Index:     1
            CRC32:              0x38e2fed9
            Metadata Size:      120 bytes
            Descriptor Offset:  32
    rc=0

    ===== FIXED: identical redundant copies (regression check) =====
    FWU Metadata:
            Version:            2
            Active Index:       0
            Previous Index:     1
            CRC32:              0x38e2fed9
            Metadata Size:      120 bytes
    rc=0

    ===== FIXED: single copy (regression check) =====
    FWU Metadata:
            Version:            2
            Active Index:       0
            Previous Index:     1
            CRC32:              0x38e2fed9
            Metadata Size:      120 bytes
    rc=0
---
 tools/fwumdata_src/fwumdata.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/tools/fwumdata_src/fwumdata.c b/tools/fwumdata_src/fwumdata.c
index 44195ce2bf2..f54a3b78d08 100644
--- a/tools/fwumdata_src/fwumdata.c
+++ b/tools/fwumdata_src/fwumdata.c
@@ -246,21 +246,35 @@ static void update_crc(struct fwu_mdata *mdata, size_t size)
 	mdata->crc32 = crc32(0, (const u8 *)&mdata->version, size - sizeof(u32));
 }
 
+/*
+ * Return a pointer to metadata copy @id inside the contiguous buffer.
+ * Copies are laid out mdata_size bytes apart, which is not the same as
+ * sizeof(struct fwu_mdata): the latter only covers the common header, so
+ * indexing mdata as an array of struct fwu_mdata would make the copies
+ * overlap. Both copies are guaranteed to share the same size (checked in
+ * parse_config()).
+ */
+static struct fwu_mdata *mdata_copy(int id)
+{
+	return (struct fwu_mdata *)((u8 *)mdata + id * devices[0].mdata_size);
+}
+
 static int read_one_metadata(int mdata_id, size_t size)
 {
+	struct fwu_mdata *md = mdata_copy(mdata_id);
 	int ret;
 
 	ret = open_device(&devices[mdata_id]);
 	if (ret)
 		return ret;
 
-	ret = read_device(&devices[mdata_id], &mdata[mdata_id], size);
+	ret = read_device(&devices[mdata_id], md, size);
 	if (ret)
 		return ret;
 
-	if (mdata[mdata_id].version != 1 && mdata[mdata_id].version != 2) {
+	if (md->version != 1 && md->version != 2) {
 		fprintf(stderr, "Invalid metadata %d version: %u\n",
-			mdata_id, mdata[mdata_id].version);
+			mdata_id, md->version);
 	}
 
 	return 0;
@@ -283,12 +297,12 @@ static int read_metadata(bool update)
 	if (ret)
 		return ret;
 
-	if (validate_crc(&mdata[0], alloc_size) < 0) {
+	if (validate_crc(mdata_copy(0), alloc_size) < 0) {
 		fprintf(stderr,
 			"Warning: Primary metadata CRC validation failed\n");
 		mdata_mod = update;
 	} else {
-		valid_mdata = &mdata[0];
+		valid_mdata = mdata_copy(0);
 	}
 
 	if (have_redundant) {
@@ -296,16 +310,17 @@ static int read_metadata(bool update)
 		if (ret)
 			return ret;
 
-		if (validate_crc(&mdata[1], alloc_size) < 0) {
+		if (validate_crc(mdata_copy(1), alloc_size) < 0) {
 			fprintf(stderr,
 				"Warning: Secondary metadata CRC validation failed\n");
 			mdata_mod = update;
-		} else if (valid_mdata && mdata[0].crc32 != mdata[1].crc32) {
+		} else if (valid_mdata &&
+			   mdata_copy(0)->crc32 != mdata_copy(1)->crc32) {
 			fprintf(stderr,
 				"Metadatas valid but not equal, use first one as default\n");
 			mdata_mod = update;
 		} else {
-			valid_mdata = &mdata[1];
+			valid_mdata = mdata_copy(1);
 		}
 	}
 
@@ -313,7 +328,7 @@ static int read_metadata(bool update)
 		fprintf(stderr,
 			"No metadata valid, use first one as default\n");
 		mdata_mod = update;
-		valid_mdata = &mdata[0];
+		valid_mdata = mdata_copy(0);
 	}
 
 	if (valid_mdata->version == 2) {

---
base-commit: 768a9cfb7e4f072b683ef6fd621a6f463168e952
change-id: 20260818-fix_tools_fwumdata-cbef1098ff7e

Best regards,
--  
Patrice Chotard <[email protected]>