Re: [PATCH v3 2/3] efi: qemu: arm64: Add efi_rng_protocol implementation for the platform

On 12/27/19 3:26 PM, Sughosh Ganu wrote:

Add support for the EFI_RNG_PROTOCOL routines for the qemu arm64 platform. EFI_RNG_PROTOCOL is an uefi boottime service which is invoked by the efi stub in the kernel for getting random seed for kaslr.

The routines are platform specific, and use the virtio-rng device on the platform to get random data.

The feature can be enabled through the following config CONFIG_EFI_RNG_PROTOCOL

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

Changes since V2:

• Based on review comments from Heinrich Schuchardt, the rng drivers read all the bytes requested in the individual drivers. Corresponding changes made in getrng routine to remove the loop to read the bytes requested, since that would be handled in the drivers.

  board/emulation/qemu-arm/qemu-arm.c | 41 +++++++++++++++++++ include/efi_rng.h | 32 +++++++++++++++ lib/efi_loader/Kconfig | 8 ++++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_rng.c | 80 +++++++++++++++++++++++++++++++++++++ 5 files changed, 162 insertions(+) create mode 100644 include/efi_rng.h create mode 100644 lib/efi_loader/efi_rng.c

diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index e1f4709..1dcf830 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -91,3 +91,44 @@ void *board_fdt_blob_setup(void) /* QEMU loads a generated DTB for us at the start of RAM. */ return (void *)CONFIG_SYS_SDRAM_BASE; }

+#if defined(CONFIG_EFI_RNG_PROTOCOL) +#include <efi_loader.h> +#include <efi_rng.h>

+#include <dm/device-internal.h>

+efi_status_t platform_get_rng_device(struct udevice **dev) +{

• int ret;
• efi_status_t status = EFI_DEVICE_ERROR;
• struct udevice *bus, *devp;
• for (uclass_first_device(UCLASS_VIRTIO, &bus); bus;

•     uclass_next_device(&bus)) {
  

• for (device_find_first_child(bus, &devp); devp; device_find_next_child(&devp)) {
  

• 	if (device_get_uclass_id(devp) == UCLASS_RNG) {
  

• 		*dev = devp;
  

• 		status = EFI_SUCCESS;
  

• 		break;
  

• 	}
  

• }
  

• }
• if (status != EFI_SUCCESS) {

• debug("No rng device found\n");
  

• return EFI_DEVICE_ERROR;
  

• }
• if (*dev) {

• ret = device_probe(*dev);
  

• if (ret)
  

• 	return EFI_DEVICE_ERROR;
  

• } else {

• debug("Couldn't get child device\n");
  

• return EFI_DEVICE_ERROR;
  

• }
• return EFI_SUCCESS;

+} +#endif /* CONFIG_EFI_RNG_PROTOCOL */ diff --git a/include/efi_rng.h b/include/efi_rng.h new file mode 100644 index 0000000..a46e66d --- /dev/null +++ b/include/efi_rng.h @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2019, Linaro Limited
• */

+#if !defined _EFI_RNG_H_ +#define _EFI_RNG_H_

+#include <efi.h> +#include <efi_api.h>

+/* EFI random number generation protocol related GUID definitions */ +#define EFI_RNG_PROTOCOL_GUID \

• EFI_GUID(0x3152bca5, 0xeade, 0x433d, 0x86, 0x2e, \

•  0xc0, 0x1c, 0xdc, 0x29, 0x1f, 0x44)
  

+#define EFI_RNG_ALGORITHM_RAW \

• EFI_GUID(0xe43176d7, 0xb6e8, 0x4827, 0xb7, 0x84, \

•  0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61)
  

+struct efi_rng_protocol {

• efi_status_t (EFIAPI *get_info)(struct efi_rng_protocol *protocol,

• 			efi_uintn_t *rng_algorithm_list_size,
  

• 			efi_guid_t *rng_algorithm_list);
  

• efi_status_t (EFIAPI *get_rng)(struct efi_rng_protocol *protocol,

• 		       efi_guid_t *rng_algorithm,
  

• 		       efi_uintn_t rng_value_length, uint8_t *rng_value);
  

+};

+efi_status_t platform_get_rng_device(struct udevice **dev);

+#endif /* _EFI_RNG_H_ */ diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 21ef440..24dde6f 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -120,4 +120,12 @@ config EFI_GRUB_ARM32_WORKAROUND GRUB prior to version 2.04 requires U-Boot to disable caches. This workaround currently is also needed on systems with caches that cannot be managed via CP15.

+config EFI_RNG_PROTOCOL

• bool "EFI_RNG_PROTOCOL support"
• depends on DM_RNG
• help

•  "Support for EFI_RNG_PROTOCOL implementation. Uses the rng
  

•   device on the platform"
  

• endif

diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 7db4060..04dc864 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,3 +42,4 @@ obj-$(CONFIG_PARTITIONS) += efi_disk.o obj-$(CONFIG_NET) += efi_net.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o +obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o diff --git a/lib/efi_loader/efi_rng.c b/lib/efi_loader/efi_rng.c new file mode 100644 index 0000000..eb91aa7 --- /dev/null +++ b/lib/efi_loader/efi_rng.c @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2019, Linaro Limited
• */

+#include <common.h> +#include <dm.h> +#include <efi_loader.h> +#include <efi_rng.h> +#include <rng.h>

+DECLARE_GLOBAL_DATA_PTR;

+static efi_status_t EFIAPI rng_getinfo(struct efi_rng_protocol *this,

• 		       efi_uintn_t *rng_algorithm_list_size,
  

• 		       efi_guid_t *rng_algorithm_list)
  

+{

• efi_guid_t rng_algo_guid = EFI_RNG_ALGORITHM_RAW;
• EFI_ENTRY("%p, %p, %p", this, rng_algorithm_list_size,

•   rng_algorithm_list);
  

• if (!this || !rng_algorithm_list_size)

• return EFI_INVALID_PARAMETER;
  

You have to call EFI_EXIT() when returning to restore the gd register.

• if (!rng_algorithm_list ||

•    *rng_algorithm_list_size < sizeof(*rng_algorithm_list)) {
  

• *rng_algorithm_list_size = sizeof(*rng_algorithm_list);
  

• return EFI_BUFFER_TOO_SMALL;
  

EFI_EXIT()

• }
• /*

• * For now, use EFI_RNG_ALGORITHM_RAW as the default
  

• * algorithm. If a new algorithm gets added in the
  

• * future through a Kconfig, rng_algo_guid will be set
  

• * based on that Kconfig option
  

• */
  

• *rng_algorithm_list_size = sizeof(*rng_algorithm_list);
• guidcpy(rng_algorithm_list, &rng_algo_guid);
• return EFI_EXIT(EFI_SUCCESS);

+}

+static efi_status_t EFIAPI getrng(struct efi_rng_protocol *this,

• 		  efi_guid_t *rng_algorithm,
  

• 		  efi_uintn_t rng_value_length,
  

• 		  uint8_t *rng_value)
  

+{

• int ret;
• struct udevice *dev;
• const efi_guid_t rng_raw_guid = EFI_RNG_ALGORITHM_RAW;
• EFI_ENTRY("%p, %p, %zu, %p", this, rng_algorithm, rng_value_length,

•   rng_value);
  

• if (!this || !rng_value || !rng_value_length)

• return EFI_INVALID_PARAMETER;
  

You have to call EFI_EXIT() when returning to get the gd register restored.

In most other protocol implementations we have here just

ret = EFI_INVALID_PARAMETER; goto out;

And a label at the end of the code.

• if (rng_algorithm) {

• if (guidcmp(rng_algorithm, &rng_raw_guid))
  

Let's have some debug output here:

EFI_PRINT("RNG algorithm %pUl\n", rng_algorthm);

• 	return EFI_UNSUPPORTED;
  

EFI_EXIT()

• }
• ret = platform_get_rng_device(&dev);
• if (ret != EFI_SUCCESS)

EFI_SUCCESS is what we use for return values of type efi_status_t. It is misleading here.

• return EFI_UNSUPPORTED;
  

EFI_EXIT()

• ret = dm_rng_read(dev, rng_value, rng_value_length);
• if (ret < 0) {

• debug("Rng device read failed\n");
  

It is preferable to use EFI_PRINT() here to have correct indentation.

• return EFI_DEVICE_ERROR;
  

EFI_EXIT()

• }
• return EFI_EXIT(EFI_SUCCESS);

+}

+const struct efi_rng_protocol efi_rng_protocol = {

• .get_info = rng_getinfo,
• .get_rng = getrng,

+};

So I think we will have to get to something like:

static efi_status_t EFIAPI rng_getinfo(struct efi_rng_protocol *this, efi_uintn_t *rng_algorithm_list_size, efi_guid_t *rng_algorithm_list) { efi_guid_t rng_algo_guid = EFI_RNG_ALGORITHM_RAW; efi_status_t ret = EFI_SUCCESS;

EFI_ENTRY("%p, %p, %p", this, rng_algorithm_list_size, rng_algorithm_list);

if (!this || !rng_algorithm_list_size) { ret = EFI_INVALID_PARAMETER; goto out; }

if (!rng_algorithm_list || *rng_algorithm_list_size < sizeof(*rng_algorithm_list)) { *rng_algorithm_list_size = sizeof(*rng_algorithm_list); ret = EFI_BUFFER_TOO_SMALL; goto out; }

/* * For now, use EFI_RNG_ALGORITHM_RAW as the default * algorithm. If a new algorithm gets added in the * future through a Kconfig, rng_algo_guid will be set * based on that Kconfig option */ *rng_algorithm_list_size = sizeof(*rng_algorithm_list); guidcpy(rng_algorithm_list, &rng_algo_guid);

out: return EFI_EXIT(ret); }

static efi_status_t EFIAPI getrng(struct efi_rng_protocol *this, efi_guid_t *rng_algorithm, efi_uintn_t rng_value_length, uint8_t *rng_value) { int r; efi_status_t ret = EFI_SUCCESS; struct udevice *dev; const efi_guid_t rng_raw_guid = EFI_RNG_ALGORITHM_RAW;

EFI_ENTRY("%p, %p, %zu, %p", this, rng_algorithm, rng_value_length, rng_value);

if (!this || !rng_value || !rng_value_length) { ret = EFI_INVALID_PARAMETER; goto out; }

if (rng_algorithm) { EFI_PRINT("RNG algorithm %pUl\n", rng_algorithm); if (guidcmp(rng_algorithm, &rng_raw_guid)) { ret = EFI_UNSUPPORTED; goto out; } }

r = platform_get_rng_device(&dev); if (r) { ret = EFI_UNSUPPORTED; goto out; }

ret = dm_rng_read(dev, rng_value, rng_value_length); if (ret < 0) { EFI_PRINT("Rng device read failed\n"); ret = EFI_DEVICE_ERROR; }

out: return EFI_EXIT(ret); }

Best regards

Heinrich