From: Andreas Reichel <[email protected]> asprintf may fail to allocate memory and return -1. Don't ignore return values and do proper error handling.
Check malloc and calloc calls to not return NULL. Simplify journal processing in bg_setenv to simplify memory management. Signed-off-by: Andreas Reichel <[email protected]> --- tools/bg_setenv.c | 112 +++++++++++++++++++++++++++++++++--------------------- tools/ebgpart.c | 51 +++++++++++++++++++------ 2 files changed, 108 insertions(+), 55 deletions(-) diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c index 58687f4..4878226 100644 --- a/tools/bg_setenv.c +++ b/tools/bg_setenv.c @@ -65,31 +65,52 @@ struct env_action { STAILQ_HEAD(stailhead, env_action) head = STAILQ_HEAD_INITIALIZER(head); -static void journal_add_action(BGENV_TASK task, char *key, char *type, - uint8_t *data, size_t datalen) +static void journal_free_action(struct env_action *action) +{ + if (!action) + return; + free(action->data); + free(action->type); + free(action->key); + free(action); +} + +static error_t journal_add_action(BGENV_TASK task, char *key, char *type, + uint8_t *data, size_t datalen) { struct env_action *new_action; new_action = calloc(1, sizeof(struct env_action)); + if (!new_action) { + return ENOMEM; + } new_action->task = task; if (key) { - asprintf(&(new_action->key), "%s", key); + if (asprintf(&(new_action->key), "%s", key) == -1) { + new_action->key = NULL; + goto newaction_nomem; + } } if (type) { - asprintf(&(new_action->type), "%s", type); + if (asprintf(&(new_action->type), "%s", type) == -1) { + new_action->type = NULL; + goto newaction_nomem; + } } if (data && datalen) { new_action->data = (uint8_t *)malloc(datalen); + if (!new_action->data) { + new_action->data = NULL; + goto newaction_nomem; + } memcpy(new_action->data, data, datalen); } STAILQ_INSERT_TAIL(&head, new_action, journal); -} + return 0; -static void journal_free_action(struct env_action *action) -{ - if (action->data) free(action->data); - if (action->type) free(action->type); - if (action->key) free(action->key); +newaction_nomem: + journal_free_action(new_action); + return ENOMEM; } static void journal_process_action(BGENV *env, struct env_action *action) @@ -175,7 +196,7 @@ static char *ustate2str(uint8_t ustate) return ustatemap[ustate]; } -static int set_uservars(char *arg) +static error_t set_uservars(char *arg) { char *key, *value; @@ -186,21 +207,18 @@ static int set_uservars(char *arg) value = strtok(NULL, "="); if (value == NULL) { - journal_add_action(ENV_TASK_DEL, key, NULL, NULL, 0); - return 0; + return journal_add_action(ENV_TASK_DEL, key, NULL, NULL, 0); } - - journal_add_action(ENV_TASK_SET, key, USERVAR_TYPE_DEFAULT, - (uint8_t *)value, strlen(value) + 1); - - return 0; + return journal_add_action(ENV_TASK_SET, key, USERVAR_TYPE_DEFAULT, + (uint8_t *)value, strlen(value) + 1); } static error_t parse_opt(int key, char *arg, struct argp_state *state) { struct arguments *arguments = state->input; - int i; + int i, res; char *tmp; + error_t e = 0; switch (key) { case 'k': @@ -211,8 +229,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) ENV_STRING_LENGTH); return 1; } - journal_add_action(ENV_TASK_SET, "kernelfile", "String", - (uint8_t *)arg, strlen(arg) + 1); + e = journal_add_action(ENV_TASK_SET, "kernelfile", "String", + (uint8_t *)arg, strlen(arg) + 1); break; case 'a': if (strlen(arg) > ENV_STRING_LENGTH) { @@ -222,8 +240,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) ENV_STRING_LENGTH); return 1; } - journal_add_action(ENV_TASK_SET, "kernelparams", "String", - (uint8_t *)arg, strlen(arg) + 1); + e = journal_add_action(ENV_TASK_SET, "kernelparams", "String", + (uint8_t *)arg, strlen(arg) + 1); break; case 'p': i = strtol(arg, &tmp, 10); @@ -263,9 +281,12 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) ustatemap[3]); return 1; } else { - asprintf(&tmp, "%u", i); - journal_add_action(ENV_TASK_SET, "ustate", "String", - (uint8_t *)tmp, strlen(tmp) + 1); + res = asprintf(&tmp, "%u", i); + if (res == -1) { + return ENOMEM; + } + e = journal_add_action(ENV_TASK_SET, "ustate", "String", + (uint8_t *)tmp, strlen(tmp) + 1); VERBOSE(stdout, "Ustate set to %d (%s).\n", i, ustate2str(i)); } @@ -273,17 +294,18 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) case 'r': i = atoi(arg); VERBOSE(stdout, "Revision is set to %d.\n", i); - journal_add_action(ENV_TASK_SET, "revision", "String", - (uint8_t *)arg, strlen(arg) + 1); + e = journal_add_action(ENV_TASK_SET, "revision", "String", + (uint8_t *)arg, strlen(arg) + 1); break; case 'w': i = atoi(arg); if (i != 0) { VERBOSE(stdout, "Setting watchdog timeout to %d seconds.\n", i); - journal_add_action(ENV_TASK_SET, "watchdog_timeout_sec", - "String", (uint8_t *)arg, - strlen(arg) + 1); + e = journal_add_action(ENV_TASK_SET, + "watchdog_timeout_sec", + "String", (uint8_t *)arg, + strlen(arg) + 1); } else { fprintf(stderr, "Watchdog timeout must be non-zero.\n"); return 1; @@ -291,14 +313,17 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) break; case 'f': arguments->output_to_file = true; - asprintf(&envfilepath, "%s/%s", arg, FAT_ENV_FILENAME); + res = asprintf(&envfilepath, "%s/%s", arg, FAT_ENV_FILENAME); + if (res == -1) { + return ENOMEM; + } break; case 'c': VERBOSE(stdout, "Confirming environment to work. Removing boot-once " "and testing flag.\n"); - journal_add_action(ENV_TASK_SET, "ustate", "String", - (uint8_t *)"0", 2); + e = journal_add_action(ENV_TASK_SET, "ustate", "String", + (uint8_t *)"0", 2); break; case 'u': if (part_specified) { @@ -318,7 +343,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) break; case 'x': /* Set user-defined variable(s) */ - set_uservars(arg); + e = set_uservars(arg); break; case 'V': printf("EFI Boot Guard %s\n", EFIBOOTGUARD_VERSION); @@ -331,7 +356,11 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) default: return ARGP_ERR_UNKNOWN; } - return 0; + + if (e) { + fprintf(stderr, "Error creating journal: %s\n", strerror(e)); + } + return e; } static void dump_uservars(uint8_t *udata) @@ -374,21 +403,16 @@ static void update_environment(BGENV *env) printf("Processing journal...\n"); - STAILQ_FOREACH(action, &head, journal) - { + while (!STAILQ_EMPTY(&head)) { + action = STAILQ_FIRST(&head); journal_process_action(env, action); + STAILQ_REMOVE_HEAD(&head, journal); journal_free_action(action); - STAILQ_REMOVE(&head, action, env_action, journal); } env->data->crc32 = crc32(0, (const Bytef *)env->data, sizeof(BG_ENVDATA) - sizeof(env->data->crc32)); - while (!STAILQ_EMPTY(&head)) { - action = STAILQ_FIRST(&head); - STAILQ_REMOVE_HEAD(&head, journal); - free(action); - } } static void dump_envs(void) diff --git a/tools/ebgpart.c b/tools/ebgpart.c index ed8a0bb..d5b5de3 100644 --- a/tools/ebgpart.c +++ b/tools/ebgpart.c @@ -76,7 +76,9 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e, if (strcmp(GPT_PARTITION_GUID_FAT_NTFS, GUID_to_str(e->type_GUID)) != 0 && strcmp(GPT_PARTITION_GUID_ESP, GUID_to_str(e->type_GUID)) != 0) { - asprintf(&pfst->name, "%s", "not supported"); + if (asprintf(&pfst->name, "%s", "not supported") == -1) { + goto error_asprintf; + } return true; } VERBOSE(stdout, "GPT Partition #%u is FAT/NTFS.\n", i); @@ -117,11 +119,17 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e, } } if (strcmp(FAT_id, "FAT12 ") == 0) { - asprintf(&pfst->name, "%s", "fat12"); + if (asprintf(&pfst->name, "%s", "fat12") == -1) { + goto error_asprintf; + } } else if (strcmp(FAT_id, "FAT16 ") == 0) { - asprintf(&pfst->name, "%s", "fat16"); + if (asprintf(&pfst->name, "%s", "fat16") == -1) { + goto error_asprintf; + } } else { - asprintf(&pfst->name, "%s", "fat32"); + if (asprintf(&pfst->name, "%s", "fat32") == -1) { + goto error_asprintf; + } } VERBOSE(stdout, "GPT Partition #%u is %s.\n", i, pfst->name); if (lseek64(fd, curr, SEEK_SET) == -1) { @@ -130,6 +138,10 @@ static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e, return false; } return true; + +error_asprintf: + VERBOSE(stderr, "Error in asprintf - possibly out of memory.\n"); + return false; } static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num, @@ -329,7 +341,9 @@ static bool check_partition_table(PedDevice *dev) list_end = &((*list_end)->next); if (t == MBR_TYPE_EXTENDED || t == MBR_TYPE_EXTENDED_LBA) { - asprintf(&pfst->name, "%s", "extended"); + if (asprintf(&pfst->name, "%s", "extended") == -1) { + goto cpt_out_of_mem; + } scanLogicalVolumes(fd, 0, &mbr, i, tmp, 5); /* Could be we still have MBR entries after * logical volumes */ @@ -337,11 +351,14 @@ static bool check_partition_table(PedDevice *dev) list_end = &((*list_end)->next); } } else { - asprintf(&pfst->name, "%s", type_to_name(t)); + if (asprintf(&pfst->name, "%s", type_to_name(t)) == -1) { + goto cpt_out_of_mem; + } } continue; cpt_out_of_mem: close(fd); + VERBOSE(stderr, "Out of mem while checking partition table\n."); if (pfst) free(pfst); if (tmp) free(tmp); return false; @@ -451,15 +468,27 @@ void ped_device_probe_all(void) } /* This is a block device, so add it to the list*/ PedDevice *dev = calloc(sizeof(PedDevice), 1); - asprintf(&dev->model, "%s", "N/A"); - asprintf(&dev->path, "%s", fullname); + if (!dev) { + continue; + } + if (asprintf(&dev->model, "%s", "N/A") == -1) { + dev->model = NULL; + goto pedprobe_error; + } + if (asprintf(&dev->path, "%s", fullname) == -1) { + dev->path = NULL; + goto pedprobe_error; + } if (check_partition_table(dev)) { add_block_dev(dev); - } else { + continue; + } +pedprobe_error: + if (dev->model) free(dev->model); + if (dev->path) free(dev->path); - free(dev); - } + free(dev); } while (sysblockfile); closedir(sysblockdir); -- 2.14.2 -- You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/20171018112937.17238-1-andreas.reichel.ext%40siemens.com. For more options, visit https://groups.google.com/d/optout.
