From 23ad52fff4da99895b40a0dddb63579adaae2358 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Thu, 12 Sep 2019 13:52:35 +0900 Subject: [PATCH 1/6] efi_loader: device_path: support Sandbox's "host" devices Sandbox's "host" devices are currently described as UCLASS_ROOT udevice with DEV_IF_HOST block device. As the current implementation of efi_device_path doesn't support such a type, any "host" device on sandbox cannot be seen as a distinct object. For example, => host bind 0 /foo/disk.img => efi devices Scanning disk host0... Found 1 disks Device Device Path ================ ==================== 0000000015c19970 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) 0000000015c19d70 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) => efi dh Handle Protocols ================ ==================== 0000000015c19970 Device Path, Device Path To Text, Device Path Utilities, Unicode Collation 2, HII String, HII Database, HII Config Routing 0000000015c19ba0 Driver Binding 0000000015c19c10 Simple Text Output 0000000015c19c80 Simple Text Input, Simple Text Input Ex 0000000015c19d70 Block IO, Device Path, Simple File System As you can see here, efi_root (0x0000000015c19970) and host0 device (0x0000000015c19d70) have the same representation of device path. This is not only inconvenient, but also confusing since two different efi objects are associated with the same device path and efi_dp_find_obj() will possibly return a wrong result. Solution: Each "host" device should be given an additional device path node of "vendor device path" to make it distinguishable. Signed-off-by: AKASHI Takahiro Reviewed-by: Heinrich Schuchardt Signed-off-by: Heinrich Schuchardt --- include/efi_loader.h | 8 ++++++++ lib/efi_loader/efi_device_path.c | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/include/efi_loader.h b/include/efi_loader.h index dd24a2746c..53b369945b 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -24,6 +24,10 @@ #define U_BOOT_GUID \ EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \ 0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b) +/* GUID used as host device on sandbox */ +#define U_BOOT_HOST_DEV_GUID \ + EFI_GUID(0xbbe4e671, 0x5773, 0x4ea1, \ + 0x9a, 0xab, 0x3a, 0x7d, 0xbf, 0x40, 0xc4, 0x82) /* Root node */ extern efi_handle_t efi_root; @@ -121,6 +125,10 @@ uint16_t *efi_dp_str(struct efi_device_path *dp); /* GUID of the U-Boot root node */ extern const efi_guid_t efi_u_boot_guid; +#ifdef CONFIG_SANDBOX +/* GUID of U-Boot host device on sandbox */ +extern const efi_guid_t efi_guid_host_dev; +#endif /* GUID of the EFI_BLOCK_IO_PROTOCOL */ extern const efi_guid_t efi_block_io_guid; extern const efi_guid_t efi_global_variable_guid; diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ea39f13b73..86297bb7c1 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -12,8 +12,13 @@ #include #include #include +#include #include +#ifdef CONFIG_SANDBOX +const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID; +#endif + /* template END node: */ static const struct efi_device_path END = { .type = DEVICE_PATH_TYPE_END, @@ -445,6 +450,16 @@ static unsigned dp_size(struct udevice *dev) case UCLASS_MMC: return dp_size(dev->parent) + sizeof(struct efi_device_path_sd_mmc_path); +#endif +#ifdef CONFIG_SANDBOX + case UCLASS_ROOT: + /* + * Sandbox's host device will be represented + * as vendor device with extra one byte for + * device number + */ + return dp_size(dev->parent) + + sizeof(struct efi_device_path_vendor) + 1; #endif default: return dp_size(dev->parent); @@ -505,6 +520,24 @@ static void *dp_fill(void *buf, struct udevice *dev) #ifdef CONFIG_BLK case UCLASS_BLK: switch (dev->parent->uclass->uc_drv->id) { +#ifdef CONFIG_SANDBOX + case UCLASS_ROOT: { + /* stop traversing parents at this point: */ + struct efi_device_path_vendor *dp = buf; + struct blk_desc *desc = dev_get_uclass_platdata(dev); + + dp_fill(buf, dev->parent); + dp = buf; + ++dp; + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; + dp->dp.length = sizeof(*dp) + 1; + memcpy(&dp->guid, &efi_guid_host_dev, + sizeof(efi_guid_t)); + dp->vendor_data[0] = desc->devnum; + return &dp->vendor_data[1]; + } +#endif #ifdef CONFIG_IDE case UCLASS_IDE: { struct efi_device_path_atapi *dp = From 7dc10c933c5d95d173dd17c8c69be8585146c4c8 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 13 Sep 2019 18:20:40 +0200 Subject: [PATCH 2/6] efi_loader: incorrect return value form DisconnectController DisconnectController() should never return EFI_NOT_FOUND. If EFI_DRIVER_BINDING_PROTOCOL.Stop() fails, return EFI_DEVICE_ERROR. If the driver handle does not expose the EFI_DRIVER_BINDING_PROTOCOL return EFI_INVALID_PARAMETER. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_boottime.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b9bff894cb..493d906c64 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -3499,7 +3499,6 @@ static efi_status_t EFIAPI efi_disconnect_controller( efi_handle_t *child_handle_buffer = NULL; size_t number_of_children = 0; efi_status_t r; - size_t stop_count = 0; struct efi_object *efiobj; EFI_ENTRY("%p, %p, %p", controller_handle, driver_image_handle, @@ -3539,32 +3538,35 @@ static efi_status_t EFIAPI efi_disconnect_controller( (void **)&binding_protocol, driver_image_handle, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL)); - if (r != EFI_SUCCESS) + if (r != EFI_SUCCESS) { + r = EFI_INVALID_PARAMETER; goto out; + } /* Remove the children */ if (number_of_children) { r = EFI_CALL(binding_protocol->stop(binding_protocol, controller_handle, number_of_children, child_handle_buffer)); - if (r == EFI_SUCCESS) - ++stop_count; + if (r != EFI_SUCCESS) { + r = EFI_DEVICE_ERROR; + goto out; + } } /* Remove the driver */ - if (!child_handle) + if (!child_handle) { r = EFI_CALL(binding_protocol->stop(binding_protocol, controller_handle, 0, NULL)); - if (r == EFI_SUCCESS) - ++stop_count; + if (r != EFI_SUCCESS) { + r = EFI_DEVICE_ERROR; + goto out; + } + } EFI_CALL(efi_close_protocol(driver_image_handle, &efi_guid_driver_binding_protocol, driver_image_handle, NULL)); - - if (stop_count) - r = EFI_SUCCESS; - else - r = EFI_NOT_FOUND; + r = EFI_SUCCESS; out: if (!child_handle) free(child_handle_buffer); From 6d2f27c5fd60c1310884c873350f1edc7d831bb0 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Fri, 6 Sep 2019 15:09:52 +0900 Subject: [PATCH 3/6] efi_loader: variable: support APPEND_WRITE If EFI_VARIABLE_APPEND_WRITE is specified in attributes at efi_set_variable(), specified data will be appended to the variable's original value. Attributes other than APPEND_WRITE should not be modified. With this patch, APPEND_WRITE test in 'variables' selftest will pass. Signed-off-by: AKASHI Takahiro --- lib/efi_loader/efi_variable.c | 70 ++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 6687b69a40..48ee255f87 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -424,17 +424,17 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_uintn_t data_size, const void *data) { char *native_name = NULL, *val = NULL, *s; + const char *old_val; + size_t old_size; efi_status_t ret = EFI_SUCCESS; u32 attr; EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes, data_size, data); - /* TODO: implement APPEND_WRITE */ if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) && - !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)) || - (attributes & EFI_VARIABLE_APPEND_WRITE)) { + !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) { ret = EFI_INVALID_PARAMETER; goto out; } @@ -445,35 +445,51 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, #define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS) - if ((data_size == 0) || !(attributes & ACCESS_ATTR)) { - /* delete the variable: */ - env_set(native_name, NULL); - ret = EFI_SUCCESS; - goto out; - } + old_val = env_get(native_name); + if (old_val) { + old_val = parse_attr(old_val, &attr); - val = env_get(native_name); - if (val) { - parse_attr(val, &attr); - - /* We should not free val */ - val = NULL; + /* check read-only first */ if (attr & READ_ONLY) { ret = EFI_WRITE_PROTECTED; goto out; } - /* - * attributes won't be changed - * TODO: take care of APPEND_WRITE once supported - */ - if (attr != attributes) { + if ((data_size == 0) || !(attributes & ACCESS_ATTR)) { + /* delete the variable: */ + env_set(native_name, NULL); + ret = EFI_SUCCESS; + goto out; + } + + /* attributes won't be changed */ + if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) { ret = EFI_INVALID_PARAMETER; goto out; } + + if (attributes & EFI_VARIABLE_APPEND_WRITE) { + if (!prefix(old_val, "(blob)")) { + return EFI_DEVICE_ERROR; + goto out; + } + old_size = strlen(old_val); + } else { + old_size = 0; + } + } else { + if ((data_size == 0) || !(attributes & ACCESS_ATTR) || + (attributes & EFI_VARIABLE_APPEND_WRITE)) { + /* delete, but nothing to do */ + ret = EFI_NOT_FOUND; + goto out; + } + + old_size = 0; } - val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1); + val = malloc(old_size + 2 * data_size + + strlen("{ro,run,boot,nv}(blob)") + 1); if (!val) { ret = EFI_OUT_OF_RESOURCES; goto out; @@ -481,10 +497,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, s = val; - /* - * store attributes - * TODO: several attributes are not supported - */ + /* store attributes */ attributes &= (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS); @@ -505,8 +518,13 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, } s += sprintf(s, "}"); + if (old_size) + /* APPEND_WRITE */ + s += sprintf(s, old_val); + else + s += sprintf(s, "(blob)"); + /* store payload: */ - s += sprintf(s, "(blob)"); s = bin2hex(s, data, data_size); *s = '\0'; From 5a24239c951e8dc3177b63881631868b8d487920 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Fri, 6 Sep 2019 15:09:53 +0900 Subject: [PATCH 4/6] efi_loader: selftest: enable APPEND_WRITE tests Now that APPEND_WRITE is supported, the result check for the only existing test case should be changed to 'todo' to 'error', while two more test cases are added. Signed-off-by: AKASHI Takahiro Signed-off-by: Heinrich Schuchardt --- lib/efi_selftest/efi_selftest_variables.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/efi_selftest/efi_selftest_variables.c b/lib/efi_selftest/efi_selftest_variables.c index 06c1a032dd..a6b41d1f00 100644 --- a/lib/efi_selftest/efi_selftest_variables.c +++ b/lib/efi_selftest/efi_selftest_variables.c @@ -21,6 +21,9 @@ static const efi_guid_t guid_vendor0 = static const efi_guid_t guid_vendor1 = EFI_GUID(0xff629290, 0x1fc1, 0xd73f, 0x8f, 0xb1, 0x32, 0xf9, 0x0c, 0xa0, 0x42, 0xea); +static const efi_guid_t guid_global = + EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, + 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c); /* * Setup unit test. @@ -116,7 +119,7 @@ static int execute(void) EFI_VARIABLE_APPEND_WRITE, 7, v + 8); if (ret != EFI_SUCCESS) { - efi_st_todo("SetVariable(APPEND_WRITE) failed\n"); + efi_st_error("SetVariable(APPEND_WRITE) failed\n"); } else { len = EFI_ST_MAX_DATA_SIZE; ret = runtime->get_variable(L"efi_st_var1", &guid_vendor1, @@ -131,6 +134,21 @@ static int execute(void) if (memcmp(data, v, len)) efi_st_todo("GetVariable returned wrong value\n"); } + /* Append variable 2 */ + ret = runtime->set_variable(L"efi_none", &guid_vendor1, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_APPEND_WRITE, + 15, v); + if (ret != EFI_NOT_FOUND) + efi_st_error("SetVariable(APPEND_WRITE) with size 0 to non-existent variable returns wrong code\n"); + /* Append variable 3 */ + ret = runtime->set_variable(L"PlatformLangCodes", &guid_global, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_APPEND_WRITE, + 15, v); + if (ret != EFI_WRITE_PROTECTED) + efi_st_todo("SetVariable(APPEND_WRITE) to read-only variable returns wrong code\n"); /* Enumerate variables */ boottime->set_mem(&guid, 16, 0); *varname = 0; From f8062c963a6285ce04b75570a1beaec27a40aec1 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 18 Sep 2019 10:26:29 +0900 Subject: [PATCH 5/6] lib: charset: add u16_strcmp() u16 version of strcmp(): u16_strncmp() works like u16_strcmp() but only at most n characters (in u16) are compared. This function will be used in my UEFI secure boot patch. Signed-off-by: AKASHI Takahiro Reviewed-by: Heinrich Schuchardt --- include/charset.h | 15 +++++++++++++++ lib/charset.c | 25 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/include/charset.h b/include/charset.h index 020f8a90df..fde6bddbc2 100644 --- a/include/charset.h +++ b/include/charset.h @@ -168,6 +168,21 @@ s32 utf_to_lower(const s32 code); */ s32 utf_to_upper(const s32 code); +/* + * u16_strncmp() - compare two u16 string + * + * @s1: first string to compare + * @s2: second string to compare + * @n: maximum number of u16 to compare + * Return: 0 if the first n u16 are the same in s1 and s2 + * < 0 if the first different u16 in s1 is less than the + * corresponding u16 in s2 + * > 0 if the first different u16 in s1 is greater than the + * corresponding u16 in s2 + */ +int u16_strncmp(const u16 *s1, const u16 *s2, size_t n); +#define u16_strcmp(s1, s2) u16_strncmp((s1), (s2), SIZE_MAX) + /** * u16_strlen - count non-zero words * diff --git a/lib/charset.c b/lib/charset.c index 72d745da4f..1c6a7f693d 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -335,6 +335,31 @@ s32 utf_to_upper(const s32 code) return ret; } +/* + * u16_strncmp() - compare two u16 string + * + * @s1: first string to compare + * @s2: second string to compare + * @n: maximum number of u16 to compare + * Return: 0 if the first n u16 are the same in s1 and s2 + * < 0 if the first different u16 in s1 is less than the + * corresponding u16 in s2 + * > 0 if the first different u16 in s1 is greater than the + * corresponding u16 in s2 + */ +int u16_strncmp(const u16 *s1, const u16 *s2, size_t n) +{ + int ret = 0; + + for (; n; --n, ++s1, ++s2) { + ret = *s1 - *s2; + if (ret || !*s1) + break; + } + + return ret; +} + size_t u16_strlen(const void *in) { const char *pos = in; From 79907a4f8429aef161add59b3d026cf2c734c1aa Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 18 Sep 2019 10:26:30 +0900 Subject: [PATCH 6/6] test: add tests for u16_strcmp() New seven test cases for u16_strcmp() are added under Unicode unit test, which should be executed by "ut unicode" command. Signed-off-by: AKASHI Takahiro Reviewed-by: Heinrich Schuchardt --- test/unicode_ut.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/unicode_ut.c b/test/unicode_ut.c index 1ccd36e7c9..8875cdc6b2 100644 --- a/test/unicode_ut.c +++ b/test/unicode_ut.c @@ -567,6 +567,19 @@ static int unicode_test_utf_to_upper(struct unit_test_state *uts) } UNICODE_TEST(unicode_test_utf_to_upper); +static int unicode_test_u16_strncmp(struct unit_test_state *uts) +{ + ut_assert(u16_strncmp(L"abc", L"abc", 3) == 0); + ut_assert(u16_strncmp(L"abcdef", L"abcghi", 3) == 0); + ut_assert(u16_strncmp(L"abcdef", L"abcghi", 6) < 0); + ut_assert(u16_strncmp(L"abcghi", L"abcdef", 6) > 0); + ut_assert(u16_strcmp(L"abc", L"abc") == 0); + ut_assert(u16_strcmp(L"abcdef", L"deghi") < 0); + ut_assert(u16_strcmp(L"deghi", L"abcdef") > 0); + return 0; +} +UNICODE_TEST(unicode_test_u16_strncmp); + int do_ut_unicode(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { struct unit_test *tests = ll_entry_start(struct unit_test, unicode_test);