From 1bc44d6c4fe0498f8ed2c52164f034ba483ccfb2 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 Oct 2014 10:35:56 -0400 Subject: [PATCH 20/31] Don't error on unset BootOrder when we're trying to add to or rm from it. Also print some better error messases here and there. Signed-off-by: Peter Jones --- README | 1 - src/efibootmgr/efibootmgr.c | 126 +++++++++++++++++++++++++++----------------- src/lib/efi.c | 95 +++++++++++++++++++-------------- src/man/man8/efibootmgr.8 | 3 -- 4 files changed, 131 insertions(+), 94 deletions(-) diff --git a/README b/README index 3bc0a26..edbce4b 100644 --- a/README +++ b/README @@ -29,7 +29,6 @@ usage: efibootmgr [options] -O | --delete-bootorder delete BootOrder -p | --part part (defaults to 1) containing loader -q | --quiet be quiet - --test filename don't write to NVRAM, write to filename -t | --timeout seconds Boot manager timeout -T | --delete-timeout delete Timeout value -u | --unicode | --UCS-2 pass extra args as UCS-2 (default is ASCII) diff --git a/src/efibootmgr/efibootmgr.c b/src/efibootmgr/efibootmgr.c index 4d80f87..eb13942 100644 --- a/src/efibootmgr/efibootmgr.c +++ b/src/efibootmgr/efibootmgr.c @@ -33,6 +33,7 @@ #define _GNU_SOURCE #include +#include #include #include #include @@ -123,7 +124,7 @@ read_vars(char **namelist, } return; err: - fprintf(stderr, "efibootmgr: %m\n"); + warn("efibootmgr"); exit(1); } @@ -242,16 +243,20 @@ make_boot_var(list_t *boot_list) free_number = opts.bootnum; } - if (free_number == -1) + if (free_number == -1) { + warn("efibootmgr: no available boot variables"); return NULL; + } /* Create a new efi_variable_t object and populate it. */ boot = calloc(1, sizeof(*boot)); - if (!boot) + if (!boot) { + warn("efibootmgr"); return NULL; + } if (make_linux_load_option(&boot->data, &boot->data_size) < 0) goto err_boot_entry; if (append_extra_args(&boot->data, &boot->data_size) < 0) @@ -260,8 +265,10 @@ make_boot_var(list_t *boot_list) boot->num = free_number; boot->guid = EFI_GLOBAL_VARIABLE; rc = asprintf(&boot->name, "Boot%04X", free_number); - if (rc < 0) + if (rc < 0) { + warn("efibootmgr"); goto err_boot_entry; + } boot->attributes = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS; @@ -272,8 +279,12 @@ make_boot_var(list_t *boot_list) list_add_tail(&boot->list, boot_list); return boot; err_boot_entry: - if (boot->name) + if (boot->name) { + warn("Could not set variable %s", boot->name); free(boot->name); + } else { + warn("Could not set variable"); + } if (boot->data) free(boot->data); free(boot); @@ -313,6 +324,15 @@ read_boot_order(efi_variable_t **boot_order) } static int +set_boot_u16(const char *name, uint16_t num) +{ + return efi_set_variable(EFI_GLOBAL_GUID, name, (uint8_t *)&num, + sizeof (num), EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS); +} + +static int add_to_boot_order(uint16_t num) { efi_variable_t *boot_order = NULL; @@ -321,8 +341,11 @@ add_to_boot_order(uint16_t num) int rc; rc = read_boot_order(&boot_order); - if (rc < 0) + if (rc < 0) { + if (errno == ENOENT) + rc = set_boot_u16("BootOrder", num); return rc; + } /* We've now got an array (in boot_order->data) of the * boot order. First add our entry, then copy the old array. @@ -358,8 +381,11 @@ remove_dupes_from_boot_order(void) int rc; rc = read_boot_order(&boot_order); - if (rc < 0) + if (rc < 0) { + if (errno == ENOENT) + rc = 0; return rc; + } old_data = (uint16_t *)(boot_order->data); /* Start with the same size */ @@ -409,8 +435,11 @@ remove_from_boot_order(uint16_t num) int rc; rc = read_boot_order(&boot_order); - if (rc < 0) + if (rc < 0) { + if (errno == ENOENT) + rc = 0; return rc; + } /* We've now got an array (in boot_order->data) of the boot order. Simply copy the array, skipping the @@ -470,15 +499,6 @@ read_boot_u16(const char *name) } static int -set_boot_u16(const char *name, uint16_t num) -{ - return efi_set_variable(EFI_GLOBAL_GUID, name, (uint8_t *)&num, - sizeof (num), EFI_VARIABLE_NON_VOLATILE | - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS); -} - -static int delete_boot_var(uint16_t num) { int rc; @@ -490,13 +510,18 @@ delete_boot_var(uint16_t num) rc = efi_del_variable(EFI_GLOBAL_GUID, name); /* For backwards compatibility, try to delete abcdef entries as well */ - if (rc < 0 && errno == ENOENT) { - snprintf(name, sizeof(name), "Boot%04x", num); - rc = efi_del_variable(EFI_GLOBAL_GUID, name); + if (rc < 0) { + if (errno == ENOENT) { + snprintf(name, sizeof(name), "Boot%04x", num); + rc = efi_del_variable(EFI_GLOBAL_GUID, name); + } else if (errno == EPERM) { + warn("Could not delete Boot%04X", num); + return rc; + } } if (rc < 0) { - fprintf(stderr,"\nboot entry: %X not found\n\n",num); + warnx("Boot entry %04X not found", num); return rc; } list_for_each_safe(pos, n, &boot_entry_list) { @@ -512,7 +537,6 @@ delete_boot_var(uint16_t num) return 0; } - static void set_var_nums(list_t *list) { @@ -1177,34 +1201,28 @@ main(int argc, char **argv) if (opts.iface && ( opts.acpi_hid < 0 || opts.acpi_uid < 0 || opts.acpi_hid > UINT32_MAX || - opts.acpi_uid > UINT32_MAX)) { - fprintf(stderr, "\nYou must specify the ACPI HID and UID when using -i.\n\n"); - return 1; - } + opts.acpi_uid > UINT32_MAX)) + errx(1, "You must specify the ACPI HID and UID when using -i."); - if (!efi_variables_supported()) { - fprintf(stderr, "\nEFI variables are not supported on this system.\n\n"); - return 1; - } + if (!efi_variables_supported()) + errx(2, "EFI variables are not supported on this system."); read_boot_var_names(&boot_names); read_vars(boot_names, &boot_entry_list); set_var_nums(&boot_entry_list); if (opts.delete_boot) { - if (opts.bootnum == -1) { - fprintf(stderr, "\nYou must specify a boot entry to delete (see the -b option).\n\n"); - return 1; - } + if (opts.bootnum == -1) + errx(3, "You must specify a boot entry to delete " + "(see the -b option)."); else ret = delete_boot_var(opts.bootnum); } if (opts.active >= 0) { - if (opts.bootnum == -1) { - fprintf(stderr, "\nYou must specify a boot entry to activate (see the -b option).\n\n"); - return 1; - } + if (opts.bootnum == -1) + errx(4, "You must specify a boot entry to activate " + "(see the -b option"); else ret=set_active_state(); } @@ -1212,47 +1230,57 @@ main(int argc, char **argv) if (opts.create) { warn_duplicate_name(&boot_entry_list); new_boot = make_boot_var(&boot_entry_list); - if (!new_boot) { - fprintf(stderr, "\nCould not prepare boot variable: %m\n\n"); - return 1; - } + if (!new_boot) + err(5, "Could not prepare boot variable"); /* Put this boot var in the right BootOrder */ if (new_boot) ret=add_to_boot_order(new_boot->num); + if (ret) + err(6, "Could not add entry to BootOrder"); } if (opts.delete_bootorder) { ret = efi_del_variable(EFI_GLOBAL_GUID, "BootOrder"); + err(7, "Could not remove entry from BootOrder"); } if (opts.bootorder) { ret = set_boot_order(opts.keep_old_entries); + if (ret) + err(8, "Could not set BootOrder"); } if (opts.deduplicate) { ret = remove_dupes_from_boot_order(); + if (ret) + err(9, "Could not set BootOrder"); } if (opts.delete_bootnext) { ret = efi_del_variable(EFI_GLOBAL_GUID, "BootNext"); + if (ret) + err(10, "Could not set BootNext"); } if (opts.delete_timeout) { ret = efi_del_variable(EFI_GLOBAL_GUID, "Timeout"); + if (ret) + err(11, "Could not delete Timeout"); } if (opts.bootnext >= 0) { - if (!is_current_boot_entry(opts.bootnext & 0xFFFF)){ - fprintf (stderr,"\n\nboot entry %X does not exist\n\n", - opts.bootnext); - return 1; - } - ret=set_boot_u16("BootNext", opts.bootnext & 0xFFFF); + if (!is_current_boot_entry(opts.bootnext & 0xFFFF)) + errx(12, "Boot entry %X does not exist", opts.bootnext); + ret = set_boot_u16("BootNext", opts.bootnext & 0xFFFF); + if (ret) + err(13, "Could not set BootNext"); } if (opts.set_timeout) { - ret=set_boot_u16("Timeout", opts.timeout); + ret = set_boot_u16("Timeout", opts.timeout); + if (ret) + err(14, "Could not set Timeout"); } if (!opts.quiet && ret == 0) { diff --git a/src/lib/efi.c b/src/lib/efi.c index 7cdc884..d19c00d 100644 --- a/src/lib/efi.c +++ b/src/lib/efi.c @@ -19,6 +19,7 @@ */ #include +#include #include #include #include @@ -502,7 +503,6 @@ static ssize_t make_disk_load_option(char *disk, uint8_t *buf, size_t size) { int disk_fd=0; - char buffer[80]; char signature[16]; int rc, edd_version=0; uint8_t mbr_type=0, signature_type=0; @@ -514,11 +514,8 @@ make_disk_load_option(char *disk, uint8_t *buf, size_t size) memset(signature, 0, sizeof(signature)); disk_fd = open(opts.disk, O_RDWR); - if (disk_fd == -1) { - sprintf(buffer, "Could not open disk %s", opts.disk); - perror(buffer); - return -1; - } + if (disk_fd == -1) + err(5, "Could not open disk %s", opts.disk); if (opts.edd_version) { edd_version = get_edd_version(); @@ -539,12 +536,10 @@ make_disk_load_option(char *disk, uint8_t *buf, size_t size) &part_start, &part_size, signature, &mbr_type, &signature_type); close(disk_fd); - if (rc) { - fprintf(stderr, "Error: no partition information on disk %s.\n" - " Cowardly refusing to create a boot option.\n", + if (rc) + errx(5, "No partition information on disk %s.\n" + "Cowardly refusing to create a boot option.\n", opts.disk); - return -1; - } needed = make_harddrive_device_path(opts.part, part_start, part_size, (uint8_t *)signature, mbr_type, @@ -724,10 +719,13 @@ make_linux_load_option(uint8_t **data, size_t *data_size) uint8_t *buf; ssize_t needed; off_t buf_offset = 0, desc_offset; + int rc; load_option = calloc(1, sizeof (*load_option)); - if (load_option == NULL) + if (load_option == NULL) { + fprintf(stderr, "efibootmgr: %m\n"); return -1; + } buf = (uint8_t *)load_option; buf_offset = 0; @@ -755,21 +753,33 @@ make_linux_load_option(uint8_t **data, size_t *data_size) if (opts.iface) { needed = make_net_load_option(opts.iface, NULL, 0); if (needed < 0) { + fprintf(stderr, "efibootmgr: could not create load option: %m\n"); free(buf); return needed; } buf = extend(load_option, load_option_size, needed); - make_net_load_option(opts.iface, buf + buf_offset, needed); + rc = make_net_load_option(opts.iface, buf + buf_offset, needed); buf_offset += needed; + if (rc < 0) { + fprintf(stderr, "efibootmgr: could not create load option: %m\n"); + free(buf); + return rc; + } } else { needed = make_disk_load_option(opts.iface, NULL, 0); if (needed < 0) { + fprintf(stderr, "efibootmgr: could not create load option: %m\n"); free(buf); return needed; } buf = extend(load_option, load_option_size, needed); - make_disk_load_option(opts.iface, buf + buf_offset, needed); + rc = make_disk_load_option(opts.iface, buf + buf_offset, needed); buf_offset += needed; + if (rc < 0) { + fprintf(stderr, "efibootmgr: could not create load option: %m\n"); + free(buf); + return rc; + } } load_option->file_path_list_length = buf_offset - desc_offset; @@ -792,25 +802,25 @@ append_extra_args_ascii(uint8_t **data, size_t *data_size) int i; unsigned long usedchars=0; - if (!data || *data) + if (!data || *data) { + errno = EINVAL; return -1; + } for (i=opts.optind; i < opts.argc; i++) { - int l = strlen(opts.argv[i]) + 1; + int l = strlen(opts.argv[i]); int space = (i < opts.argc - 1) ? 1: 0; - uint8_t *tmp = realloc(new_data, (usedchars + l + space)); + uint8_t *tmp = realloc(new_data, (usedchars + l + space + 1)); if (tmp == NULL) return -1; new_data = tmp; p = (char *)new_data + usedchars; strcpy(p, opts.argv[i]); usedchars += l; - p += l; /* Put a space between args */ if (space) - p[usedchars++] = ' '; - else - p[usedchars] = '\0'; + new_data[usedchars++] = ' '; + new_data[usedchars] = '\0'; } if (!new_data) @@ -829,8 +839,10 @@ append_extra_args_unicode(uint8_t **data, size_t *data_size) int i; unsigned long usedchars=0; - if (!data || *data) + if (!data || *data) { + errno = EINVAL; return -1; + } for (i = opts.optind; i < opts.argc; i++) { int l = strlen(opts.argv[i]) + 1; @@ -871,37 +883,31 @@ append_extra_args_file(uint8_t **data, size_t *data_size) size_t maxchars = 0; char *buffer; - if (!data) { - fprintf(stderr, "internal error\n"); - exit(1); + if (!data || *data) { + errno = EINVAL; + return -1; } if (file && strncmp(file, "-", 1)) fd = open(file, O_RDONLY); - if (fd == -1) { - perror("Failed to open extra arguments file"); - exit(1); - } + if (fd < 0) + return -1; buffer = malloc(maxchars); do { if (maxchars - appended == 0) { maxchars += 1024; char *tmp = realloc(buffer, maxchars); - if (tmp == NULL) { - perror("Error reading extra arguments file"); - exit(1); - } + if (tmp == NULL) + return -1; buffer = tmp; } num_read = read(fd, buffer + appended, maxchars - appended); - if (num_read < 0) { - perror("Error reading extra arguments file"); - exit(1); - } else if (num_read > 0) { + if (num_read < 0) + return -1; + else if (num_read > 0) appended += num_read; - } } while (num_read > 0); if (fd != STDIN_FILENO) @@ -935,14 +941,18 @@ append_extra_args(uint8_t **data, size_t *data_size) if (opts.extra_opts_file) { ret = append_extra_args_file(&new_data, &new_data_size); - if (ret < 0) + if (ret < 0) { + fprintf(stderr, "efibootmgr: append_extra_args: %m\n"); return -1; + } } if (new_data_size) { ret = add_new_data(data, data_size, new_data, new_data_size); free(new_data); - if (ret < 0) + if (ret < 0) { + fprintf(stderr, "efibootmgr: append_extra_args: %m\n"); return -1; + } new_data = NULL; new_data_size = 0; } @@ -952,6 +962,7 @@ append_extra_args(uint8_t **data, size_t *data_size) else ret = append_extra_args_ascii(&new_data, &new_data_size); if (ret < 0) { + fprintf(stderr, "efibootmgr: append_extra_args: %m\n"); if (new_data) /* this can't happen, but covscan believes */ free(new_data); return -1; @@ -960,8 +971,10 @@ append_extra_args(uint8_t **data, size_t *data_size) ret = add_new_data(data, data_size, new_data, new_data_size); free(new_data); new_data = NULL; - if (ret < 0) + if (ret < 0) { + fprintf(stderr, "efibootmgr: append_extra_args: %m\n"); return -1; + } new_data_size = 0; } diff --git a/src/man/man8/efibootmgr.8 b/src/man/man8/efibootmgr.8 index 96071d7..423bc16 100644 --- a/src/man/man8/efibootmgr.8 +++ b/src/man/man8/efibootmgr.8 @@ -93,9 +93,6 @@ Partition number containing the bootloader (defaults to 1) \fB-q | --quiet\fR Quiet mode - supresses output. .TP -\fB--test \fIfilename\fB\fR -Don't write to NVRAM, write to \fIfilename\fR\&. -.TP \fB-t | --timeout \fIseconds\fB\fR Boot Manager timeout, in \fIseconds\fR\&. .TP -- 2.7.4