Skip to content

Commit

Permalink
Null-terminate 'arguments' in fallback
Browse files Browse the repository at this point in the history
In case CSV entry contains boot argument (e.g. an image to load for shim)
it must be null-terminated. While populate_stanza() makes sure 'arguments'
end with '\0', add_boot_option() doesn't account for it in 'size'
calculations. E.g. for the following CSV entry:

 shimx64.efi,6.6.0-0.rc0.20230904git708283abf896.6.fc40.x86_64,\EFI\Linux\5f93b3c9cf1c488a99786fb8e99fb840-6.6.0-0.rc0.20230904git708283abf896.6.fc40.x86_64.efi,Comment

the resulting variable after 'fallback' looks like:

 # hexdump /sys/firmware/efi/efivars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c | tail -3
 0000180 0038 0036 005f 0036 0034 002e 0065 0066
 0000190 0069
 0000192

Add trailing '\0' to 'size' calculations in add_boot_option() when
'arguments' is not empty. The resulting variable looks like:

 # hexdump /sys/firmware/efi/efivars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c | tail -3
 0000180 0038 0036 005f 0036 0034 002e 0065 0066
 0000190 0069 0000
 0000194

and the specified image is loaded by shim without issues.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
  • Loading branch information
vittyvk authored and vathpela committed Nov 12, 2024
1 parent 47bbb5e commit 338fded
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions fallback.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,11 @@ add_boot_option(EFI_DEVICE_PATH *hddp, EFI_DEVICE_PATH *fulldp,

void *var = LibGetVariable(varname, &GV_GUID);
if (!var) {
int arg_size = StrLen(arguments) ? StrLen(arguments) * sizeof (CHAR16) +
sizeof (CHAR16) : 0;
int size = sizeof(UINT32) + sizeof (UINT16) +
StrLen(label)*2 + 2 + DevicePathSize(hddp) +
StrLen(arguments) * 2;
arg_size;

CHAR8 *data, *cursor;
cursor = data = AllocateZeroPool(size + 2);
Expand All @@ -252,7 +254,7 @@ add_boot_option(EFI_DEVICE_PATH *hddp, EFI_DEVICE_PATH *fulldp,
if (!first_new_option) {
first_new_option = DuplicateDevicePath(fulldp);
first_new_option_args = StrDuplicate(arguments);
first_new_option_size = StrLen(arguments) * sizeof (CHAR16);
first_new_option_size = arg_size;
}

efi_status = RT->SetVariable(varname, &GV_GUID,
Expand Down Expand Up @@ -400,9 +402,11 @@ find_boot_option(EFI_DEVICE_PATH *dp, EFI_DEVICE_PATH *fulldp,
UINT16 *optnum)
{
unsigned int label_size = StrLen(label)*2 + 2;
int arg_size = StrLen(arguments) ? StrLen(arguments) * sizeof (CHAR16) +
sizeof (CHAR16) : 0;
unsigned int size = sizeof(UINT32) + sizeof (UINT16) +
label_size + DevicePathSize(dp) +
StrLen(arguments) * 2;
arg_size;

CHAR8 *data = AllocateZeroPool(size + 2);
if (!data)
Expand Down Expand Up @@ -486,7 +490,7 @@ find_boot_option(EFI_DEVICE_PATH *dp, EFI_DEVICE_PATH *fulldp,
if (!first_new_option) {
first_new_option = DuplicateDevicePath(fulldp);
first_new_option_args = StrDuplicate(arguments);
first_new_option_size = StrLen(arguments) * sizeof (CHAR16);
first_new_option_size = arg_size;
}

*optnum = xtoi(varname + 4);
Expand Down

0 comments on commit 338fded

Please sign in to comment.