[PATCH 4/8 v2] efi_loader: Introduce helper functions for EFI

[Heinrich Schuchardt]

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 at 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