| Message ID | 20201230150722.154663-5-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Change logic of EFI LoadFile2 protocol for initrd loading | expand |
On 12/30/20 4:07 PM, Ilias Apalodimas wrote: > A following patch introduces a different logic for loading initrd's > based on the EFI_LOAD_FILE2_PROTOCOL. > Since similar logic can be applied in the future for other system files > (i.e DTBs), let's add some helper functions which will retrieve and > parse file paths stored in EFI variables. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > include/efi_helper.h | 28 ++++++ > lib/efi_loader/efi_helper.c | 180 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 208 insertions(+) > create mode 100644 include/efi_helper.h > create mode 100644 lib/efi_loader/efi_helper.c > > diff --git a/include/efi_helper.h b/include/efi_helper.h > new file mode 100644 > index 000000000000..5d3a43364619 > --- /dev/null > +++ b/include/efi_helper.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2020, Linaro Limited > + */ > + > +#if !defined _EFI_HELPER_H_ > +#define _EFI_HELPER_H > + > +#include <efi.h> > +#include <efi_api.h> > + > +/* > + * @dev: device string i.e 'mmc' > + * @part: partition string i.e '0:2' > + * @filename: name of the file > + */ > +struct load_file_info { > + char dev[32]; if_typename_str[] contains all available strings. 8 bytes are long enough. The value can be validated with blk_get_device_by_str(). > + char part[16]; Windows allows 128 partitions per device. I would not expect millions of devices. 8 bytes should be long enough. > + char filename[256]; This is a path not a file name. Please, adjust the parameter name. In Windows the maximum length of a path is 260 characters. But does such a limit exist in UEFI? In U-Boot we have: include/ext_common.h:33:#define EXT2_PATH_MAX 4096 So you cannot assume that the path is shorter then 4096 bytes. I think the best thing to do is to use strdup() and then put 0x00 at the end of each string part. How about: struct load_file_info { /* * duplicated filespec, to be freed after usage * 0x00 separated device, part, path */ char *filespec; char *part; char *filepath; } So filespec would point to a buffer containing: device\0 part\0 \path\file\0 \0 | | |- filepath points here | |- part points here |- filespec points here > +}; > + > +loff_t get_file_size(const struct load_file_info *file_loc, > + efi_status_t *status); > +efi_status_t efi_get_fp_from_var(const char *name, struct load_file_info *loc); > +void *get_var_efi(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size); > + > +#endif > diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c > new file mode 100644 > index 000000000000..e5919343a5cc > --- /dev/null > +++ b/lib/efi_loader/efi_helper.c > @@ -0,0 +1,180 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2020, Linaro Limited > + */ > + > +#include <common.h> > +#include <env.h> > +#include <malloc.h> > +#include <dm.h> > +#include <fs.h> > +#include <efi_helper.h> > +#include <efi_loader.h> > +#include <efi_variable.h> > + > +/** > + * get_file_size() - retrieve the size of initramfs, set efi status on error > + * > + * @dev: device to read from, e.g. "mmc" > + * @part: device partition, e.g. "0:1" > + * @file: name of file > + * @status: EFI exit code in case of failure > + * > + * Return: size of file > + */ > +loff_t get_file_size(const struct load_file_info *info, efi_status_t *status) > +{ > + loff_t sz = 0; > + int ret; > + > + ret = fs_set_blk_dev(info->dev, info->part, FS_TYPE_ANY); > + if (ret) { > + *status = EFI_NO_MEDIA; > + goto out; > + } > + > + ret = fs_size(info->filename, &sz); > + if (ret) { > + sz = 0; > + *status = EFI_NOT_FOUND; > + goto out; > + } > + > +out: > + return sz; > +} > + > +/* > + * string_to_load_args() - Fill in a struct load_file_info with the file info > + * parsed from an EFI variable > + * > + * @args: value of the EFI variable i.e "mmc 0 initrd" > + * @info: struct to fill in with file specific info > + * > + * Return: Status code > + */ > +static efi_status_t string_to_load_args(char *args, struct load_file_info *info) > +{ > + efi_status_t status = EFI_SUCCESS; > + char *p; > + > + /* > + * expect a string with three space separated parts: > + * - block device type, e.g. "mmc" > + * - device and partition identifier, e.g. "0:1" > + * - file path on the block device, e.g. "/boot/initrd.cpio.gz" > + */ Shouldn't you skip over leading spaces? > + p = strsep(&args, " "); > + if (!p) { > + status = EFI_NO_MEDIA; > + goto out; > + } > + strncpy(info->dev, p, sizeof(info->dev)); > + info->dev[sizeof(info->dev) - 1] = 0; > + > + p = strsep(&args, " "); > + if (!p) { > + status = EFI_NO_MEDIA; > + goto out; > + } Don't make any assumptions about the number of spaces between the file specification parts: " host 0:1 /dir1/dir12/filename " We should use strdup() and make_argv(). > + strncpy(info->part, p, sizeof(info->part)); > + info->part[sizeof(info->part) - 1] = 0; > + > + p = strsep(&args, " "); > + if (!p) { > + status = EFI_NOT_FOUND; > + goto out; > + } > + strncpy(info->filename, p, sizeof(info->filename)); > + info->filename[sizeof(info->filename) - 1] = 0; > + > +out: > + return status; > +} > + > +/** > + * get_var_efi() - read value of an EFI variable > + * > + * @name: variable name > + * @start: vendor GUID > + * @size: size of allocated buffer > + * > + * Return: buffer with variable data or NULL > + */ > +void *get_var_efi(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size) > +{ > + efi_status_t ret; > + void *buf = NULL; > + > + *size = 0; > + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL); > + if (ret == EFI_BUFFER_TOO_SMALL) { > + buf = malloc(*size); > + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL); > + } > + > + if (ret != EFI_SUCCESS) { > + free(buf); > + *size = 0; > + return NULL; > + } > + > + return buf; > +} > + > +/** > + * efi_get_fp_from_var() - Retrieve a file path from an EFI variable > + * > + * @name: variable name > + * @info: struct to fill in with file specific info > + */ > +efi_status_t efi_get_fp_from_var(const char *name, struct load_file_info *info) > +{ > + efi_uintn_t boot_cur_size; > + void *var_value = NULL; > + efi_uintn_t size; > + efi_status_t ret; > + u16 boot_cur; > + u16 var_name[16]; > + u16 *pos; > + > + memset(info, 0, sizeof(*info)); > + > + boot_cur_size = sizeof(boot_cur); > + ret = efi_get_variable_int(L"BootCurrent", Please, put some more text into the function description indicating the usage of BootCurrent. > + &efi_global_variable_guid, NULL, > + &boot_cur_size, &boot_cur, NULL); > + if (ret != EFI_SUCCESS) > + goto out; > + > + pos = efi_create_indexed_name(var_name, sizeof(var_name), name, > + boot_cur); > + if (!pos) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > + } > + > + var_value = get_var_efi(var_name, &efi_global_variable_guid, &size); > + if (!var_value) { > + ret = EFI_NOT_FOUND; > + goto out; > + } > + > + ret = string_to_load_args(var_value, info); > + if (ret != EFI_SUCCESS) > + goto out; > + > + if (fs_set_blk_dev(info->dev, info->part, FS_TYPE_ANY)) { > + ret = EFI_NO_MEDIA; > + goto out; > + } > + > + if (!fs_exists(info->filename)) { > + ret = EFI_NOT_FOUND; > + goto out; > + } > + > +out: > + free(var_value); > + return ret; > +} > Best regards Heinrich
On Wed, Dec 30, 2020 at 08:29:54PM +0100, Heinrich Schuchardt wrote: > On 12/30/20 4:07 PM, Ilias Apalodimas wrote: > > A following patch introduces a different logic for loading initrd's [...] > > +struct load_file_info { > > + char dev[32]; > > if_typename_str[] contains all available strings. 8 bytes are long > enough. The value can be validated with blk_get_device_by_str(). > > > + char part[16]; > > Windows allows 128 partitions per device. I would not expect millions of > devices. 8 bytes should be long enough. > > > + char filename[256]; > > This is a path not a file name. Please, adjust the parameter name. > > In Windows the maximum length of a path is 260 characters. But does such > a limit exist in UEFI? > > In U-Boot we have: > include/ext_common.h:33:#define EXT2_PATH_MAX 4096 > > So you cannot assume that the path is shorter then 4096 bytes. This is defining something entirely differnt though. So I dont think we need to adhere to any of these. 256 is perfectly sane for an initrd path. > > I think the best thing to do is to use strdup() and then put 0x00 at the > end of each string part. How about: > > struct load_file_info { > /* > * duplicated filespec, to be freed after usage > * 0x00 separated device, part, path > */ > char *filespec; > char *part; > char *filepath; > } > > So filespec would point to a buffer containing: > > device\0 part\0 \path\file\0 \0 > | | |- filepath points here > | |- part points here > |- filespec points here > I generally try to avoid functions that need free() after the usagem since they tend to be error prone. In any case this whole thing we probably go away if we just store a device path in initrd. So let me have a look and see were we'll end up. [...] Cheers /Ilias
diff --git a/include/efi_helper.h b/include/efi_helper.h new file mode 100644 index 000000000000..5d3a43364619 --- /dev/null +++ b/include/efi_helper.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2020, Linaro Limited + */ + +#if !defined _EFI_HELPER_H_ +#define _EFI_HELPER_H + +#include <efi.h> +#include <efi_api.h> + +/* + * @dev: device string i.e 'mmc' + * @part: partition string i.e '0:2' + * @filename: name of the file + */ +struct load_file_info { + char dev[32]; + char part[16]; + char filename[256]; +}; + +loff_t get_file_size(const struct load_file_info *file_loc, + efi_status_t *status); +efi_status_t efi_get_fp_from_var(const char *name, struct load_file_info *loc); +void *get_var_efi(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size); + +#endif diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c new file mode 100644 index 000000000000..e5919343a5cc --- /dev/null +++ b/lib/efi_loader/efi_helper.c @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2020, Linaro Limited + */ + +#include <common.h> +#include <env.h> +#include <malloc.h> +#include <dm.h> +#include <fs.h> +#include <efi_helper.h> +#include <efi_loader.h> +#include <efi_variable.h> + +/** + * get_file_size() - retrieve the size of initramfs, set efi status on error + * + * @dev: device to read from, e.g. "mmc" + * @part: device partition, e.g. "0:1" + * @file: name of file + * @status: EFI exit code in case of failure + * + * Return: size of file + */ +loff_t get_file_size(const struct load_file_info *info, efi_status_t *status) +{ + loff_t sz = 0; + int ret; + + ret = fs_set_blk_dev(info->dev, info->part, FS_TYPE_ANY); + if (ret) { + *status = EFI_NO_MEDIA; + goto out; + } + + ret = fs_size(info->filename, &sz); + if (ret) { + sz = 0; + *status = EFI_NOT_FOUND; + goto out; + } + +out: + return sz; +} + +/* + * string_to_load_args() - Fill in a struct load_file_info with the file info + * parsed from an EFI variable + * + * @args: value of the EFI variable i.e "mmc 0 initrd" + * @info: struct to fill in with file specific info + * + * Return: Status code + */ +static efi_status_t string_to_load_args(char *args, struct load_file_info *info) +{ + efi_status_t status = EFI_SUCCESS; + char *p; + + /* + * expect a string with three space separated parts: + * - block device type, e.g. "mmc" + * - device and partition identifier, e.g. "0:1" + * - file path on the block device, e.g. "/boot/initrd.cpio.gz" + */ + p = strsep(&args, " "); + if (!p) { + status = EFI_NO_MEDIA; + goto out; + } + strncpy(info->dev, p, sizeof(info->dev)); + info->dev[sizeof(info->dev) - 1] = 0; + + p = strsep(&args, " "); + if (!p) { + status = EFI_NO_MEDIA; + goto out; + } + strncpy(info->part, p, sizeof(info->part)); + info->part[sizeof(info->part) - 1] = 0; + + p = strsep(&args, " "); + if (!p) { + status = EFI_NOT_FOUND; + goto out; + } + strncpy(info->filename, p, sizeof(info->filename)); + info->filename[sizeof(info->filename) - 1] = 0; + +out: + return status; +} + +/** + * get_var_efi() - read value of an EFI variable + * + * @name: variable name + * @start: vendor GUID + * @size: size of allocated buffer + * + * Return: buffer with variable data or NULL + */ +void *get_var_efi(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size) +{ + efi_status_t ret; + void *buf = NULL; + + *size = 0; + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) { + buf = malloc(*size); + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL); + } + + if (ret != EFI_SUCCESS) { + free(buf); + *size = 0; + return NULL; + } + + return buf; +} + +/** + * efi_get_fp_from_var() - Retrieve a file path from an EFI variable + * + * @name: variable name + * @info: struct to fill in with file specific info + */ +efi_status_t efi_get_fp_from_var(const char *name, struct load_file_info *info) +{ + efi_uintn_t boot_cur_size; + void *var_value = NULL; + efi_uintn_t size; + efi_status_t ret; + u16 boot_cur; + u16 var_name[16]; + u16 *pos; + + memset(info, 0, sizeof(*info)); + + boot_cur_size = sizeof(boot_cur); + ret = efi_get_variable_int(L"BootCurrent", + &efi_global_variable_guid, NULL, + &boot_cur_size, &boot_cur, NULL); + if (ret != EFI_SUCCESS) + goto out; + + pos = efi_create_indexed_name(var_name, sizeof(var_name), name, + boot_cur); + if (!pos) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + + var_value = get_var_efi(var_name, &efi_global_variable_guid, &size); + if (!var_value) { + ret = EFI_NOT_FOUND; + goto out; + } + + ret = string_to_load_args(var_value, info); + if (ret != EFI_SUCCESS) + goto out; + + if (fs_set_blk_dev(info->dev, info->part, FS_TYPE_ANY)) { + ret = EFI_NO_MEDIA; + goto out; + } + + if (!fs_exists(info->filename)) { + ret = EFI_NOT_FOUND; + goto out; + } + +out: + free(var_value); + return ret; +}
A following patch introduces a different logic for loading initrd's based on the EFI_LOAD_FILE2_PROTOCOL. Since similar logic can be applied in the future for other system files (i.e DTBs), let's add some helper functions which will retrieve and parse file paths stored in EFI variables. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- include/efi_helper.h | 28 ++++++ lib/efi_loader/efi_helper.c | 180 ++++++++++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 include/efi_helper.h create mode 100644 lib/efi_loader/efi_helper.c -- 2.30.0