From f0cc7adb2fc2ecdc4e6f0dd4e81b7895859b55d2 Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Wed, 18 Nov 2020 18:31:22 -0800 Subject: [PATCH] cbfstool: Ensure attributes always come last in the metadata In a rare placement edge case when adding a file with alignment requirements, cbfstool may need to generate a CBFS header that's slightly larger than it needs to be. The way we do this is by just increasing the data offset field in the CBFS header until the data falls to the desired value. This approach works but it may confuse parsing code in the presence of CBFS attributes. Normally, the whole area between the attribute offset and the data offset is filled with valid attributes written back to back, but when this header expansion occurs the attributes are followed by some garbage data (usually 0xff). Parsers are resilient against this but may show unexpected error messages. This patch solves the problem by moving the attribute offset forwards together with the data offset, so that the total area used for attributes doesn't change. Instead, the filename field becomes the expanded area, which is a closer match to how this worked when it was originally implemented (before attributes existed) and is less confusing for parsers since filenames are zero-terminated anyway. Signed-off-by: Julius Werner Change-Id: I3dd503dd5c9e6c4be437f694a7f8993a57168c2b Reviewed-on: https://review.coreboot.org/c/coreboot/+/47824 Tested-by: build bot (Jenkins) Reviewed-by: Angel Pons Reviewed-by: Aaron Durbin --- util/cbfstool/cbfs_image.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 4249015c24..0191682de9 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -658,15 +658,28 @@ static int cbfs_add_entry_at(struct cbfs_image *image, len = content_offset - addr - header_size; memcpy(entry, header, header_size); if (len != 0) { - /* the header moved backwards a bit to accommodate cbfs_file + /* + * The header moved backwards a bit to accommodate cbfs_file * alignment requirements, so patch up ->offset to still point - * to file data. + * to file data. Move attributes forward so the end of the + * attribute list still matches the end of the metadata. */ + uint32_t offset = ntohl(entry->offset); + uint32_t attrs = ntohl(entry->attributes_offset); DEBUG("|..|header|content|... \n"); - DEBUG("before: offset=0x%x\n", ntohl(entry->offset)); - // TODO reset expanded name buffer to 0xFF. - entry->offset = htonl(ntohl(entry->offset) + len); - DEBUG("after: offset=0x%x\n", ntohl(entry->len)); + DEBUG("before: attr_offset=0x%x, offset=0x%x\n", attrs, offset); + if (attrs == 0) { + memset((uint8_t *)entry + offset, 0, len); + } else { + uint8_t *p = (uint8_t *)entry + attrs; + memmove(p + len, p, offset - attrs); + memset(p, 0, len); + attrs += len; + entry->attributes_offset = htonl(attrs); + } + offset += len; + entry->offset = htonl(offset); + DEBUG("after: attr_offset=0x%x, offset=0x%x\n", attrs, offset); } // Ready to fill data into entry.