[PATCH v5 5/7] cmd: efidebug: add uri device path
Masahisa Kojima
masahisa.kojima at linaro.org
Fri Sep 29 04:20:30 CEST 2023
On Thu, 28 Sept 2023 at 23:32, Maxim Uvarov <maxim.uvarov at linaro.org> wrote:
>
>
>
> On Wed, 27 Sept 2023 at 15:38, Masahisa Kojima <masahisa.kojima at linaro.org> wrote:
>>
>> This adds the URI device path option for 'boot add' subcommand.
>> User can add the URI load option for downloading ISO image file
>> or EFI application through network. Currently HTTP is only supported.
>>
>> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
>> ---
>> cmd/efidebug.c | 50 +++++++++++++++++++++++++++++++++++
>> include/net.h | 8 ++++++
>> net/wget.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 130 insertions(+)
>>
>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>> index 0be3af3e76..f2fd6ba71d 100644
>> --- a/cmd/efidebug.c
>> +++ b/cmd/efidebug.c
>> @@ -19,6 +19,7 @@
>> #include <log.h>
>> #include <malloc.h>
>> #include <mapmem.h>
>> +#include <net.h>
>> #include <part.h>
>> #include <search.h>
>> #include <linux/ctype.h>
>> @@ -829,6 +830,52 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag,
>> argc -= 1;
>> argv += 1;
>> break;
>> +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS))
>> + case 'u':
>> + {
>> + char *pos;
>> + int uridp_len;
>> + struct efi_device_path_uri *uridp;
>> +
>> + if (argc < 3 || lo.label) {
>> + r = CMD_RET_USAGE;
>> + goto out;
>> + }
>> + id = (int)hextoul(argv[1], &endp);
>> + if (*endp != '\0' || id > 0xffff)
>> + return CMD_RET_USAGE;
>> +
>> + efi_create_indexed_name(var_name16, sizeof(var_name16),
>> + "Boot", id);
>> +
>> + label = efi_convert_string(argv[2]);
>> + if (!label)
>> + return CMD_RET_FAILURE;
>> + lo.label = label;
>> +
>> + uridp_len = sizeof(struct efi_device_path) + strlen(argv[3]) + 1;
>> + fp_free = efi_alloc(uridp_len + sizeof(END));
>> + uridp = (struct efi_device_path_uri *)fp_free;
>> + uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
>> + uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI;
>> + uridp->dp.length = uridp_len;
>> + if (!wget_validate_uri(argv[3])) {
>> + printf("ERROR: invalid URI\n");
>> + r = CMD_RET_FAILURE;
>> + goto out;
>> + }
>> +
>> + strcpy(uridp->uri, argv[3]);
>> + pos = (char *)uridp + uridp_len;
>> + memcpy(pos, &END, sizeof(END));
>> + fp_size += uridp_len + sizeof(END);
>> + file_path = (struct efi_device_path *)uridp;
>> + argc -= 3;
>> + argv += 3;
>> + break;
>> + }
>> +#endif
>> +
>> default:
>> r = CMD_RET_USAGE;
>> goto out;
>> @@ -1492,6 +1539,9 @@ static char efidebug_help_text[] =
>> " -b|-B <bootid> <label> <interface> <devnum>[:<part>] <file path>\n"
>> " -i|-I <interface> <devnum>[:<part>] <initrd file path>\n"
>> " (-b, -i for short form device path)\n"
>> +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS))
>> + " -u <bootid> <label> <uri>\n"
>> +#endif
>> " -s '<optional data>'\n"
>> "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
>> " - delete UEFI BootXXXX variables\n"
>> diff --git a/include/net.h b/include/net.h
>> index 57889d8b7a..c748974573 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -935,4 +935,12 @@ static inline void eth_set_enable_bootdevs(bool enable) {}
>> */
>> int wget_with_dns(ulong dst_addr, char *uri);
>>
>> +/**
>> + * wget_validate_uri() - varidate the uri
>> + *
>> + * @uri: uri string of target file of wget
>> + * Return: true if uri is valid, false if uri is invalid
>> + */
>> +bool wget_validate_uri(char *uri);
>> +
>> #endif /* __NET_H__ */
>> diff --git a/net/wget.c b/net/wget.c
>> index 4801e28eb9..6a4d22be32 100644
>> --- a/net/wget.c
>> +++ b/net/wget.c
>> @@ -558,3 +558,75 @@ out:
>> return ret;
>> }
>> #endif
>> +
>> +/**
>> + * wget_validate_uri() - validate the uri for wget
>> + *
>> + * @uri: uri string
>> + * Return: true on success, false on failure
>> + */
>
>
> It's better to have a comment description in one place. You already have
> it in the header file. And even here the text is a little bit differ.
OK, I will add a comment in the function header.
>
>>
>> +bool wget_validate_uri(char *uri)
>> +{
>> + char c;
>> + bool ret = true;
>> + char *str_copy, *s, *authority;
>> +
>> + /* TODO: strict uri conformance check */
>> +
>> + /*
>> + * Uri is expected to be correctly percent encoded.
>> + * This is the minimum check, control codes(0x1-0x19, 0x7F, except '\0')
>> + * and space character(0x20) are not allowed.
>> + */
>> + for (c = 0x1; c < 0x21; c++) {
>> + if (strchr(uri, c)) {
>> + printf("invalid character is used\n");
>> + return false;
>> + }
>> + }
>> + if (strchr(uri, 0x7f)) {
>> + printf("invalid character is used\n");
>> + return false;
>> + }
>
>
> So you validate that only English characters exist in the url. There are urls with non English characters.
I only excluded the control codes and space(0x20), so non-English characters
and UTF-8 encoded URIs can be available.
>
>
>>
>> +
>> + /*
>> + * This follows the current U-Boot wget implementation.
>> + * scheme: only "http:" is supported
>> + * authority:
>> + * - user information: not supported
>> + * - host: supported
>> + * - port: not supported(always use the default port)
>> + */
>> + if (strncmp(uri, "http://", 7)) {
>> + printf("only http:// is supported\n");
>
>
> log_err()
OK, I will replace other printf() calls.
Thanks,
Masahisa Kojima
>
>>
>> + return false;
>> + }
>> + str_copy = strdup(uri);
>> + if (!str_copy)
>> + return false;
>> +
>> + s = str_copy + strlen("http://");
>> + authority = strsep(&s, "/");
>> + if (!s) {
>> + printf("invalid uri, no file path\n");
>> + ret = false;
>> + goto out;
>> + }
>> + s = strchr(authority, '@');
>> + if (s) {
>> + printf("user information is not supported\n");
>> + ret = false;
>> + goto out;
>> + }
>> + s = strchr(authority, ':');
>> + if (s) {
>> + printf("user defined port is not supported\n");
>> + ret = false;
>> + goto out;
>> + }
>> +
>> +out:
>> + free(str_copy);
>> +
>> + return ret;
>> +}
>> --
>> 2.34.1
>>
More information about the U-Boot
mailing list