See attachments
* guard all mallocs in cbfstool * fix an issue that could lead to cbfstool writing outside of its allocated memory
Signed-off-by: Stefan Reinauer stepan@coresystems.de
--- util/cbfstool/common.c (.../branches/upstream/coreboot-v2) +++ util/cbfstool/common.c (.../trunk/coreboot-v2) @@ -36,10 +36,16 @@ fseek(file, 0, SEEK_END); *romsize_p = ftell(file); fseek(file, 0, SEEK_SET); - if (!content) + if (!content) { content = malloc(*romsize_p); - else if (place == SEEK_END) + if (!content) { + printf("Could not get %d bytes for file %s\n", + *romsize_p, filename); + exit(1); + } + } else if (place == SEEK_END) content -= *romsize_p; + if (!fread(content, *romsize_p, 1, file)) { printf("failed to read %s\n", filename); return NULL; @@ -255,6 +261,11 @@ *location -= headersize; } void *newdata = malloc(*datasize + headersize); + if (!newdata) { + printf("Could not get %d bytes for CBFS file.\n", *datasize + + headersize); + exit(1); + } struct cbfs_file *nextfile = (struct cbfs_file *)newdata; strncpy(nextfile->magic, "LARCHIVE", 8); nextfile->len = htonl(*datasize); @@ -272,9 +283,16 @@ { romsize = _romsize; unsigned char *romarea = malloc(romsize); + if (!romarea) { + printf("Could not get %d bytes of memory for CBFS image.\n", + romsize); + exit(1); + } memset(romarea, 0xff, romsize); - recalculate_rom_geometry(romarea);
+ // Set up physical/virtual mapping + offset = romarea + romsize - 0x100000000ULL; + if (align == 0) align = 64;
@@ -291,6 +309,9 @@ master_header->offset = htonl(0); ((uint32_t *) phys_to_virt(0xfffffffc))[0] = virt_to_phys(master_header); + + recalculate_rom_geometry(romarea); + struct cbfs_file *one_empty_file = cbfs_create_empty_file((0 - romsize) & 0xffffffff, romsize - bootblocksize - --- util/cbfstool/common.h (.../branches/upstream/coreboot-v2) +++ util/cbfstool/common.h (.../trunk/coreboot-v2) @@ -29,7 +29,7 @@
static uint32_t virt_to_phys(void *addr) { - return (long)(addr - offset) & 0xffffffff; + return (unsigned long)(addr - offset) & 0xffffffff; }
#define ALIGN(val, by) (((val) + (by)-1)&~((by)-1)) @@ -61,3 +61,5 @@
int add_file_to_cbfs(void *content, uint32_t contentsize, uint32_t location); void print_cbfs_directory(const char *filename); + +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
This separates the code for each command in cbfstool. For the good and for the bad: It brings a certain amount of code duplication (some of which can be cleaned up again, or get rid of by proper refactoring). On the other hand now there's a very simple code flow for each command, rather than for each operation. ie.
adding a file to a cbfs means: - open the cbfs - add the file - close the cbfs
rather than
open the cbfs: - do this for add, remove, but not for create
create a new lar - if we don't have an open one yet
add a file: - if we didn't bail out before
close the file: - if we didn't bail out before
The short term benefit is that this fixes a problem where cbfstool was trying to add a file if you gave a non-existing command because it bailed out on known, not on unknown commands.
Signed-off-by: Stefan Reinauer stepan@coresystems.de
--- util/cbfstool/cbfstool.c (.../branches/upstream/coreboot-v2) +++ util/cbfstool/cbfstool.c (.../trunk/coreboot-v2) @@ -23,50 +23,79 @@ #include "common.h" #include "cbfs.h"
-int main(int argc, char **argv) +typedef enum { + CMD_ADD, + CMD_ADD_PAYLOAD, + CMD_ADD_STAGE, + CMD_CREATE, + CMD_PRINT +} cmd_t; + +struct command { + cmd_t id; + const char *name; + int (*function) (int argc, char **argv); +}; + +static int cbfs_add(int argc, char **argv) { - if (argc < 3) { - printf - ("cbfstool: Management utility for CBFS formatted ROM images\n" - "USAGE:\n" "cbfstool [-h]\n" - "cbfstool FILE COMMAND [PARAMETERS]...\n\n" "OPTIONs:\n" - " -h Display this help message\n\n" - "COMMANDs:\n" - "add FILE NAME TYPE [base address] Add a component\n" - "add-payload FILE NAME [COMP] [base] Add a payload to the ROM\n" - "add-stage FILE NAME [COMP] [base] Add a stage to the ROM\n" - "create SIZE BSIZE BOOTBLOCK [ALIGN] Create a ROM file\n" - "print Show the contents of the ROM\n"); - return 1; - } char *romname = argv[1]; char *cmd = argv[2]; + void *rom = loadrom(romname);
- if (strcmp(cmd, "create") == 0) { - if (argc < 6) { - printf("not enough arguments to 'create'.\n"); - return 1; - } - uint32_t size = strtoul(argv[3], NULL, 0); - /* ignore bootblock size. we use whatever we get and won't allocate any larger */ - char *bootblock = argv[5]; - uint32_t align = 0; - if (argc > 6) - align = strtoul(argv[6], NULL, 0); - return create_cbfs_image(romname, size, bootblock, align); + if (rom == NULL) { + printf("Could not load ROM image '%s'.\n", romname); + return 1; }
+ if (argc < 5) { + printf("not enough arguments to '%s'.\n", cmd); + return 1; + } + + char *filename = argv[3]; + char *cbfsname = argv[4]; + + uint32_t filesize = 0; + void *filedata = loadfile(filename, &filesize, 0, SEEK_SET); + if (filedata == NULL) { + printf("Could not load file '%s'.\n", filename); + return 1; + } + + uint32_t base = 0; + void *cbfsfile = NULL; + + if (argc < 6) { + printf("not enough arguments to 'add'.\n"); + return 1; + } + uint32_t type; + if (intfiletype(argv[5]) != ((uint64_t) - 1)) + type = intfiletype(argv[5]); + else + type = strtoul(argv[5], NULL, 0); + if (argc > 6) { + base = strtoul(argv[6], NULL, 0); + } + cbfsfile = + create_cbfs_file(cbfsname, filedata, &filesize, type, &base); + add_file_to_cbfs(cbfsfile, filesize, base); + writerom(romname, rom, romsize); + return 0; +} + +static int cbfs_add_payload(int argc, char **argv) +{ + char *romname = argv[1]; + char *cmd = argv[2]; void *rom = loadrom(romname); + if (rom == NULL) { printf("Could not load ROM image '%s'.\n", romname); return 1; }
- if (strcmp(cmd, "print") == 0) { - print_cbfs_directory(romname); - return 0; - } - if (argc < 5) { printf("not enough arguments to '%s'.\n", cmd); return 1; @@ -83,59 +112,150 @@ }
uint32_t base = 0; - void *cbfsfile; + void *cbfsfile = NULL;
- if (strcmp(cmd, "add") == 0) { - if (argc < 6) { - printf("not enough arguments to 'add'.\n"); - return 1; - } - uint32_t type; - if (intfiletype(argv[5]) != ((uint64_t) - 1)) - type = intfiletype(argv[5]); - else - type = strtoul(argv[5], NULL, 0); - if (argc > 6) { - base = strtoul(argv[6], NULL, 0); - } - cbfsfile = - create_cbfs_file(cbfsname, filedata, &filesize, type, - &base); + comp_algo algo = CBFS_COMPRESS_NONE; + if (argc > 5) { + if (argv[5][0] == 'l') + algo = CBFS_COMPRESS_LZMA; } + if (argc > 6) { + base = strtoul(argv[6], NULL, 0); + } + unsigned char *payload; + filesize = parse_elf_to_payload(filedata, &payload, algo); + cbfsfile = + create_cbfs_file(cbfsname, payload, &filesize, + CBFS_COMPONENT_PAYLOAD, &base); + add_file_to_cbfs(cbfsfile, filesize, base); + writerom(romname, rom, romsize); + return 0; +}
- if (strcmp(cmd, "add-payload") == 0) { - comp_algo algo = CBFS_COMPRESS_NONE; - if (argc > 5) { - if (argv[5][0] == 'l') - algo = CBFS_COMPRESS_LZMA; - } - if (argc > 6) { - base = strtoul(argv[6], NULL, 0); - } - unsigned char *payload; - filesize = parse_elf_to_payload(filedata, &payload, algo); - cbfsfile = - create_cbfs_file(cbfsname, payload, &filesize, - CBFS_COMPONENT_PAYLOAD, &base); +static int cbfs_add_stage(int argc, char **argv) +{ + char *romname = argv[1]; + char *cmd = argv[2]; + void *rom = loadrom(romname); + + if (rom == NULL) { + printf("Could not load ROM image '%s'.\n", romname); + return 1; }
- if (strcmp(cmd, "add-stage") == 0) { - comp_algo algo = CBFS_COMPRESS_NONE; - if (argc > 5) { - if (argv[5][0] == 'l') - algo = CBFS_COMPRESS_LZMA; - } - if (argc > 6) { - base = strtoul(argv[6], NULL, 0); - } - unsigned char *stage; - filesize = parse_elf_to_stage(filedata, &stage, algo, &base); - cbfsfile = - create_cbfs_file(cbfsname, stage, &filesize, - CBFS_COMPONENT_STAGE, &base); + if (argc < 5) { + printf("not enough arguments to '%s'.\n", cmd); + return 1; }
+ char *filename = argv[3]; + char *cbfsname = argv[4]; + + uint32_t filesize = 0; + void *filedata = loadfile(filename, &filesize, 0, SEEK_SET); + if (filedata == NULL) { + printf("Could not load file '%s'.\n", filename); + return 1; + } + + uint32_t base = 0; + void *cbfsfile = NULL; + + comp_algo algo = CBFS_COMPRESS_NONE; + if (argc > 5) { + if (argv[5][0] == 'l') + algo = CBFS_COMPRESS_LZMA; + } + if (argc > 6) { + base = strtoul(argv[6], NULL, 0); + } + unsigned char *stage; + filesize = parse_elf_to_stage(filedata, &stage, algo, &base); + cbfsfile = + create_cbfs_file(cbfsname, stage, &filesize, + CBFS_COMPONENT_STAGE, &base); + add_file_to_cbfs(cbfsfile, filesize, base); writerom(romname, rom, romsize); return 0; } + +static int cbfs_create(int argc, char **argv) +{ + char *romname = argv[1]; + char *cmd = argv[2]; + if (argc < 6) { + printf("not enough arguments to 'create'.\n"); + return 1; + } + + uint32_t size = strtoul(argv[3], NULL, 0); + /* ignore bootblock size. we use whatever we get and won't allocate any larger */ + char *bootblock = argv[5]; + uint32_t align = 0; + + if (argc > 6) + align = strtoul(argv[6], NULL, 0); + + return create_cbfs_image(romname, size, bootblock, align); +} + +static int cbfs_print(int argc, char **argv) +{ + char *romname = argv[1]; + char *cmd = argv[2]; + void *rom = loadrom(romname); + + if (rom == NULL) { + printf("Could not load ROM image '%s'.\n", romname); + return 1; + } + + print_cbfs_directory(romname); + return 0; +} + +struct command commands[] = { + {CMD_ADD, "add", cbfs_add}, + {CMD_ADD_PAYLOAD, "add-payload", cbfs_add_payload}, + {CMD_ADD_STAGE, "add-stage", cbfs_add_stage}, + {CMD_CREATE, "create", cbfs_create}, + {CMD_PRINT, "print", cbfs_print} +}; + +void usage(void) +{ + printf + ("cbfstool: Management utility for CBFS formatted ROM images\n" + "USAGE:\n" "cbfstool [-h]\n" + "cbfstool FILE COMMAND [PARAMETERS]...\n\n" "OPTIONs:\n" + " -h Display this help message\n\n" + "COMMANDs:\n" + "add FILE NAME TYPE [base address] Add a component\n" + "add-payload FILE NAME [COMP] [base] Add a payload to the ROM\n" + "add-stage FILE NAME [COMP] [base] Add a stage to the ROM\n" + "create SIZE BSIZE BOOTBLOCK [ALIGN] Create a ROM file\n" + "print Show the contents of the ROM\n"); +} + +int main(int argc, char **argv) +{ + int i; + + if (argc < 3) { + usage(); + return 1; + } + + char *cmd = argv[2]; + + for (i = 0; i < ARRAY_SIZE(commands); i++) { + if (strcmp(cmd, commands[i].name) != 0) + continue; + return commands[i].function(argc, argv); + } + + printf("Unknown command '%s'.\n", cmd); + usage(); + return 1; +}