mirror of
https://github.com/xen-project/xen.git
synced 2026-10-28 02:01:24 +01:00
EFI: avoid OOB config file reads
The message emitted by pre_parse() pretty clearly states the original
intention. Yet what it said wasn't done, and would have been unfriendly to
the user. Hence accesses past the allocated buffer were possible. Insert a
terminating NUL immediately past the data read, to then drop the no longer
applicable message.
NB: The iscntrl() check of just the last byte is more strict than what
pre_parse() would accept without issuing its prior message, yet I'd like
to keep the new logic reasonably simple. Config files shouldn't be huge,
and we shouldn't be _that_ short of memory (or we'd fail elsewhere pretty
soon).
Fixes: bf6501a62e ("x86-64: EFI boot code")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
This commit is contained in:
1 parent
33ceaa2827
commit
df75f77092
1 file changed
+22
-4
+22
-4
@@ -833,8 +833,9 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
|
||||
what = L"Allocation";
|
||||
file->addr = min(1UL << (32 + PAGE_SHIFT),
|
||||
HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
|
||||
/* For config files allocate an extra byte to put a NUL there. */
|
||||
ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
|
||||
PFN_UP(size), &file->addr);
|
||||
PFN_UP(size + (file == &cfg)), &file->addr);
|
||||
if ( EFI_ERROR(ret) )
|
||||
goto fail;
|
||||
|
||||
@@ -853,6 +854,9 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
|
||||
|
||||
efi_arch_flush_dcache_area(file->ptr, file->size);
|
||||
|
||||
if ( file == &cfg )
|
||||
file->str[file->size] = 0;
|
||||
|
||||
return true;
|
||||
|
||||
fail:
|
||||
@@ -878,6 +882,23 @@ static bool __init read_section(const EFI_LOADED_IMAGE *image,
|
||||
|
||||
file->ptr = ptr;
|
||||
|
||||
/* For cfg file, if necessary allocate space to put an extra NUL there. */
|
||||
if ( file == &cfg && file->size && !iscntrl(file->str[file->size - 1]) )
|
||||
{
|
||||
EFI_PHYSICAL_ADDRESS addr;
|
||||
EFI_STATUS ret = efi_bs->AllocatePages(AllocateMaxAddress,
|
||||
EfiLoaderData,
|
||||
PFN_UP(file->size + 1), &addr);
|
||||
|
||||
if ( EFI_ERROR(ret) )
|
||||
return false;
|
||||
|
||||
memcpy((void *)addr, ptr, file->size);
|
||||
file->addr = addr;
|
||||
file->need_to_free = true;
|
||||
file->str[file->size] = 0;
|
||||
}
|
||||
|
||||
handle_file_info(name, file, options);
|
||||
|
||||
return true;
|
||||
@@ -906,9 +927,6 @@ static void __init pre_parse(const struct file *file)
|
||||
else
|
||||
start = 0;
|
||||
}
|
||||
if ( file->size && end[-1] )
|
||||
PrintStr(L"No newline at end of config file,"
|
||||
" last line will be ignored.\r\n");
|
||||
}
|
||||
|
||||
static void __init init_secure_boot_mode(void)
|
||||
|
||||