Pull request for UEFI sub-system for efi-2020-04-rc4 (5)

This series contains bug fixes for the UEFI sub-system:
 
 * report correct variable length in GetNextVariable()
 * correct copying direction if freestanding memmove()
 * remove const for parameter of GetNextVariableName()
 * correct function descriptions
 
 Unit tests are added and adjusted.
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAl54VuoACgkQxIHbvCwF
 GsR1RA//bddiizl6PPaqJqykM4I6yEEv/V0skc/tPcPlpYBH5369Rda/mrIfTWfV
 FMlO0Z2qFQrKwD6qog+6/YMImSzUTH220A8yI/TItDdPODvEDRkhjQBEhBjz7FYe
 nNxrhl9A1r8IGfUztlv2mhxmIVvXEtq4cQvNOlO3bkkKue69A9E/tqCQCrSdido5
 rNIbrXSwo9LK++/U9sk9e/zTvZLW/LvwuFogiGJGhYuRi7JDMzI3OzEqfiCIgyPj
 l35xhvBxWDyZEaUKkY92lLqDuO0dMmUORW3GfLHcSqF/agTMm1i4oBn76uroHBcD
 Vb4Getu7kaBEPCreSZHdtuuUULAT5PC/iogDYS1cQE1uggpzrweC9OZUVQS/2O7I
 KeHgEdMUkLxmuXpUDGvQqoz7j73es3yM6DUUPDIRx3klNZrBOQ367ExGh9jHeS+8
 8R+iMmOcdVuwvwX0kMrZ6TwrmUtT02HFPUa7GczXT/SR06LOjKfnJnb1sswScloZ
 f7dZ1uTPLVkNhVmm3/o0qzMBaXCgu5QrDivWOqOqfYOTTPJFalVwNa4ybzB/cM5o
 4toy1SRMml1PDH3lOzDOveN0R5l46+zeRFeyemtbqDePat9z64KKAk2HM0k+d2qf
 4jP9KErghJiLzftrLtJqWqWQZlGCbEuR1SQt8O3cu6DIsUXbpLI=
 =+Nkf
 -----END PGP SIGNATURE-----

Merge tag 'efi-2020-04-rc4-5' of https://gitlab.denx.de/u-boot/custodians/u-boot-efi

Pull request for UEFI sub-system for efi-2020-04-rc4 (5)

This series contains bug fixes for the UEFI sub-system:

* report correct variable length in GetNextVariable()
* correct copying direction if freestanding memmove()
* remove const for parameter of GetNextVariableName()
* correct function descriptions

Unit tests are added and adjusted.

include/efi_api.h

@@ -272,7 +272,7 @@ struct efi_runtime_services {
 					    efi_uintn_t *data_size, void *data);
 	efi_status_t (EFIAPI *get_next_variable_name)(
 			efi_uintn_t *variable_name_size,
-			u16 *variable_name, const efi_guid_t *vendor);
+			u16 *variable_name, efi_guid_t *vendor);
 	efi_status_t (EFIAPI *set_variable)(u16 *variable_name,
 					    const efi_guid_t *vendor,
 					    u32 attributes,

include/efi_loader.h

@@ -645,7 +645,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
 				     efi_uintn_t *data_size, void *data);
 efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 					       u16 *variable_name,
-					       const efi_guid_t *vendor);
+					       efi_guid_t *vendor);
 efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
 				     const efi_guid_t *vendor, u32 attributes,
 				     efi_uintn_t data_size, const void *data);

lib/efi_loader/efi_disk.c

@@ -222,15 +222,17 @@ static const struct efi_block_io block_io_disk_template = {
 	.flush_blocks = &efi_disk_flush_blocks,
 };
 
-/*
- * Get the simple file system protocol for a file device path.
+/**
+ * efi_fs_from_path() - retrieve simple file system protocol
+ *
+ * Gets the simple file system protocol for a file device path.
  *
  * The full path provided is split into device part and into a file
  * part. The device part is used to find the handle on which the
  * simple file system protocol is installed.
  *
- * @full_path	device path including device and file
- * @return	simple file system protocol
+ * @full_path:	device path including device and file
+ * Return:	simple file system protocol
  */
 struct efi_simple_file_system_protocol *
 efi_fs_from_path(struct efi_device_path *full_path)
@@ -285,15 +287,15 @@ static int efi_fs_exists(struct blk_desc *desc, int part)
 }
 
 /*
- * Create a handle for a partition or disk
+ * efi_disk_add_dev() - create a handle for a partition or disk
  *
- * @parent	parent handle
- * @dp_parent	parent device path
- * @if_typename interface name for block device
- * @desc	internal block device
- * @dev_index   device index for block device
- * @offset	offset into disk for simple partitions
- * @return	disk object
+ * @parent:		parent handle
+ * @dp_parent:		parent device path
+ * @if_typename:	interface name for block device
+ * @desc:		internal block device
+ * @dev_index:		device index for block device
+ * @offset:		offset into disk for simple partitions
+ * Return:		disk object
  */
 static efi_status_t efi_disk_add_dev(
 				efi_handle_t parent,
@@ -365,7 +367,7 @@ static efi_status_t efi_disk_add_dev(
 	diskobj->media.block_size = desc->blksz;
 	diskobj->media.io_align = desc->blksz;
 	diskobj->media.last_block = desc->lba - offset;
-	if (part != 0)
+	if (part)
 		diskobj->media.logical_partition = 1;
 	diskobj->ops.media = &diskobj->media;
 	if (disk)
@@ -373,15 +375,17 @@ static efi_status_t efi_disk_add_dev(
 	return EFI_SUCCESS;
 }
 
-/*
- * Create handles and protocols for the partitions of a block device
+/**
+ * efi_disk_create_partitions() - create handles and protocols for partitions
  *
- * @parent		handle of the parent disk
- * @blk_desc		block device
- * @if_typename		interface type
- * @diskid		device number
- * @pdevname		device name
- * @return		number of partitions created
+ * Create handles and protocols for the partitions of a block device.
+ *
+ * @parent:		handle of the parent disk
+ * @blk_desc:		block device
+ * @if_typename:	interface type
+ * @diskid:		device number
+ * @pdevname:		device name
+ * Return:		number of partitions created
  */
 int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
 			       const char *if_typename, int diskid,
@@ -418,16 +422,20 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
 	return disks;
 }
 
-/*
+/**
+ * efi_disk_register() - register block devices
+ *
  * U-Boot doesn't have a list of all online disk devices. So when running our
  * EFI payload, we scan through all of the potentially available ones and
  * store them in our object pool.
  *
+ * This function is called in efi_init_obj_list().
+ *
  * TODO(sjg@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
  * Consider converting the code to look up devices as needed. The EFI device
  * could be a child of the UCLASS_BLK block device, perhaps.
  *
- * This gets called from do_bootefi_exec().
+ * Return:	status code
  */
 efi_status_t efi_disk_register(void)
 {

lib/efi_loader/efi_freestanding.c

@@ -47,7 +47,7 @@ void *memmove(void *dest, const void *src, size_t n)
 	u8 *d = dest;
 	const u8 *s = src;
 
-	if (d >= s) {
+	if (d <= s) {
 		for (; n; --n)
 			*d++ = *s++;
 	} else {

lib/efi_loader/efi_runtime.c

@@ -483,7 +483,7 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime(
 }
 
 /**
- * efi_convert_pointer_runtime() - convert from physical to virtual pointer
+ * efi_convert_pointer() - convert from physical to virtual pointer
  *
  * This function implements the ConvertPointer() runtime service until
  * the first call to SetVirtualAddressMap().
@@ -493,7 +493,7 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime(
  *
  * @debug_disposition:	indicates if pointer may be converted to NULL
  * @address:		pointer to be converted
- * Return:		status code EFI_UNSUPPORTED
+ * Return:		status code
  */
 static __efi_runtime efi_status_t EFIAPI efi_convert_pointer(
 			efi_uintn_t debug_disposition, void **address)

lib/efi_loader/efi_variable.c

@@ -1,8 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- *  EFI utils
+ * UEFI runtime variable services
  *
- *  Copyright (c) 2017 Rob Clark
+ * Copyright (c) 2017 Rob Clark
  */
 
 #include <common.h>
@@ -273,7 +273,8 @@ static efi_status_t parse_uboot_variable(char *variable,
 					 u32 *attributes)
 {
 	char *guid, *name, *end, c;
-	unsigned long name_len;
+	size_t name_len;
+	efi_uintn_t old_variable_name_size;
 	u16 *p;
 
 	guid = strchr(variable, '_');
@@ -289,17 +290,17 @@ static efi_status_t parse_uboot_variable(char *variable,
 		return EFI_INVALID_PARAMETER;
 
 	name_len = end - name;
-	if (*variable_name_size < (name_len + 1)) {
-		*variable_name_size = name_len + 1;
+	old_variable_name_size = *variable_name_size;
+	*variable_name_size = sizeof(u16) * (name_len + 1);
+	if (old_variable_name_size < *variable_name_size)
 		return EFI_BUFFER_TOO_SMALL;
-	}
+
 	end++; /* point to value */
 
 	/* variable name */
 	p = variable_name;
 	utf8_utf16_strncpy(&p, name, name_len);
 	variable_name[name_len] = 0;
-	*variable_name_size = name_len + 1;
 
 	/* guid */
 	c = *(name - 1);
@@ -329,7 +330,7 @@ static efi_status_t parse_uboot_variable(char *variable,
  */
 efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 					       u16 *variable_name,
-					       const efi_guid_t *vendor)
+					       efi_guid_t *vendor)
 {
 	char *native_name, *variable;
 	ssize_t name_len, list_len;
@@ -597,7 +598,7 @@ efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *vendor,
  */
 static efi_status_t __efi_runtime EFIAPI
 efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size,
-				   u16 *variable_name, const efi_guid_t *vendor)
+				   u16 *variable_name, efi_guid_t *vendor)
 {
 	return EFI_UNSUPPORTED;
 }

lib/efi_selftest/Makefile

@@ -27,6 +27,7 @@ efi_selftest_exitbootservices.o \
 efi_selftest_gop.o \
 efi_selftest_loaded_image.o \
 efi_selftest_manageprotocols.o \
+efi_selftest_mem.o \
 efi_selftest_memory.o \
 efi_selftest_open_protocol.o \
 efi_selftest_register_notify.o \

lib/efi_selftest/efi_selftest_mem.c

@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * efi_selftest_memory
+ *
+ * Copyright (c) 2018 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * This unit test checks the following boottime services:
+ * CopyMem, SetMem, CalculateCrc32
+ *
+ * The memory type used for the device tree is checked.
+ */
+
+#include <efi_selftest.h>
+
+static struct efi_boot_services *boottime;
+
+/**
+ * setup() - setup unit test
+ *
+ * @handle:	handle of the loaded image
+ * @systable:	system table
+ * Return:	EFI_ST_SUCCESS for success
+ */
+static int setup(const efi_handle_t handle,
+		 const struct efi_system_table *systable)
+{
+	boottime = systable->boottime;
+
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * execute() - execute unit test
+ *
+ * Return:	EFI_ST_SUCCESS for success
+ */
+static int execute(void)
+{
+	u8 c1[] = "abcdefghijklmnop";
+	u8 c2[] = "abcdefghijklmnop";
+	u32 crc32;
+	efi_status_t ret;
+
+	ret = boottime->calculate_crc32(c1, 16, &crc32);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("CalculateCrc32 failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (crc32 != 0x943ac093) {
+		efi_st_error("CalculateCrc32 returned wrong value\n");
+		return EFI_ST_FAILURE;
+	}
+	boottime->copy_mem(&c1[5], &c1[3], 8);
+	if (memcmp(c1, "abcdedefghijknop", 16)) {
+		efi_st_error("CopyMem forward copy failed: %s\n", c1);
+		return EFI_ST_FAILURE;
+	}
+	boottime->copy_mem(&c2[3], &c2[5], 8);
+	if (memcmp(c2, "abcfghijklmlmnop", 16)) {
+		efi_st_error("CopyMem backward copy failed: %s\n", c2);
+		return EFI_ST_FAILURE;
+	}
+	boottime->set_mem(&c1[3], 8, 'x');
+	if (memcmp(c1, "abcxxxxxxxxjknop", 16)) {
+		efi_st_error("SetMem failed: %s\n", c1);
+		return EFI_ST_FAILURE;
+	}
+
+	return EFI_ST_SUCCESS;
+}
+
+EFI_UNIT_TEST(mem) = {
+	.name = "mem",
+	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+	.setup = setup,
+	.execute = execute,
+};

lib/efi_selftest/efi_selftest_variables.c

@@ -11,7 +11,7 @@
 #include <efi_selftest.h>
 
 #define EFI_ST_MAX_DATA_SIZE 16
-#define EFI_ST_MAX_VARNAME_SIZE 40
+#define EFI_ST_MAX_VARNAME_SIZE 80
 
 static struct efi_boot_services *boottime;
 static struct efi_runtime_services *runtime;
@@ -155,8 +155,14 @@ static int execute(void)
 			return EFI_ST_FAILURE;
 		}
 		if (!memcmp(&guid, &guid_vendor0, sizeof(efi_guid_t)) &&
-		    !efi_st_strcmp_16_8(varname, "efi_st_var0"))
+		    !efi_st_strcmp_16_8(varname, "efi_st_var0")) {
 			flag |= 1;
+			if (len != 24) {
+				efi_st_error("GetNextVariableName report wrong length %u, expected 24\n",
+					     (unsigned int)len);
+				return EFI_ST_FAILURE;
+			}
+		}
 		if (!memcmp(&guid, &guid_vendor1, sizeof(efi_guid_t)) &&
 		    !efi_st_strcmp_16_8(varname, "efi_st_var1"))
 			flag |= 2;