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(a)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(a)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;
+}
