From 0290146943af203c1a9e332e19ec76b414b0a771 Mon Sep 17 00:00:00 2001 From: Zhang Ning Date: Tue, 1 Feb 2022 08:33:37 +0800 Subject: [PATCH 01/18] cmd: pxe_utils: sysboot: add kaslr-seed generation support this will add kaslrseed keyword to sysboot lable, when it set, it will request to genarate random number from hwrng as kaslr-seed. with this patch exlinux.conf label looks like label l0 menu testing linux /boot/vmlinuz-5.15.16-arm initrd /boot/initramfs-5.15.16-arm.img fdtdir /boot/dtbs/5.15.16-arm/ kaslrseed append root=UUID=92ae1e50-eeeb-4c5b-8939-7e1cd6cfb059 ro Tested on Khadas VIM with kernel 5.16.0-rc5-arm64, Debian 11. Signed-off-by: Zhang Ning --- boot/pxe_utils.c | 75 +++++++++++++++++++++++++++++++++++++++++++++ doc/README.pxe | 2 ++ include/pxe_utils.h | 2 ++ 3 files changed, 79 insertions(+) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index bb231b11a2c..0c24becae39 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -20,6 +20,11 @@ #include #include +#ifdef CONFIG_DM_RNG +#include +#include +#endif + #include #include @@ -311,6 +316,67 @@ static int label_localboot(struct pxe_label *label) return run_command_list(localcmd, strlen(localcmd), 0); } +/* + * label_boot_kaslrseed generate kaslrseed from hw rng + */ + +static void label_boot_kaslrseed(void) +{ +#ifdef CONFIG_DM_RNG + ulong fdt_addr; + struct fdt_header *working_fdt; + size_t n = 0x8; + struct udevice *dev; + u64 *buf; + int nodeoffset; + int err; + + /* Get the main fdt and map it */ + fdt_addr = hextoul(env_get("fdt_addr_r"), NULL); + working_fdt = map_sysmem(fdt_addr, 0); + err = fdt_check_header(working_fdt); + if (err) + return; + + /* add extra size for holding kaslr-seed */ + /* err is new fdt size, 0 or negtive */ + err = fdt_shrink_to_minimum(working_fdt, 512); + if (err <= 0) + return; + + if (uclass_get_device(UCLASS_RNG, 0, &dev) || !dev) { + printf("No RNG device\n"); + return; + } + + nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen"); + if (nodeoffset < 0) { + printf("Reading chosen node failed\n"); + return; + } + + buf = malloc(n); + if (!buf) { + printf("Out of memory\n"); + return; + } + + if (dm_rng_read(dev, buf, n)) { + printf("Reading RNG failed\n"); + goto err; + } + + err = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, sizeof(buf)); + if (err < 0) { + printf("Unable to set kaslr-seed on chosen node: %s\n", fdt_strerror(err)); + goto err; + } +err: + free(buf); +#endif + return; +} + /** * label_boot_fdtoverlay() - Loads fdt overlays specified in 'fdtoverlays' * @@ -631,6 +697,9 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } } + if (label->kaslrseed) + label_boot_kaslrseed(); + #ifdef CONFIG_OF_LIBFDT_OVERLAY if (label->fdtoverlays) label_boot_fdtoverlay(ctx, label); @@ -710,6 +779,7 @@ enum token_type { T_ONTIMEOUT, T_IPAPPEND, T_BACKGROUND, + T_KASLRSEED, T_INVALID }; @@ -741,6 +811,7 @@ static const struct token keywords[] = { {"ontimeout", T_ONTIMEOUT,}, {"ipappend", T_IPAPPEND,}, {"background", T_BACKGROUND,}, + {"kaslrseed", T_KASLRSEED,}, {NULL, T_INVALID} }; @@ -1194,6 +1265,10 @@ static int parse_label(char **c, struct pxe_menu *cfg) err = parse_integer(c, &label->ipappend); break; + case T_KASLRSEED: + label->kaslrseed = 1; + break; + case T_EOL: break; diff --git a/doc/README.pxe b/doc/README.pxe index a1f0423adbe..75caa01c4a1 100644 --- a/doc/README.pxe +++ b/doc/README.pxe @@ -163,6 +163,8 @@ fdtoverlays [...] - if this label is chosen, use tftp to retrieve the DT and then applied in the load order to the fdt blob stored at the address indicated in the fdt_addr_r environment variable. +kaslrseed - set this label to request random number from hwrng as kaslr seed. + append - use as the kernel command line when booting this label. diff --git a/include/pxe_utils.h b/include/pxe_utils.h index dad26688180..4a73b2aace3 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -33,6 +33,7 @@ * initrd - path to the initrd to use for this label. * attempted - 0 if we haven't tried to boot this label, 1 if we have. * localboot - 1 if this label specified 'localboot', 0 otherwise. + * kaslrseed - 1 if generate kaslrseed from hw_rng * list - lets these form a list, which a pxe_menu struct will hold. */ struct pxe_label { @@ -50,6 +51,7 @@ struct pxe_label { int attempted; int localboot; int localboot_val; + int kaslrseed; struct list_head list; }; From 80d4c02b9324e7e0049582142474c9cc8630e27c Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Wed, 2 Feb 2022 13:04:04 -0500 Subject: [PATCH 02/18] button: adc: set state to pressed when the voltage is closest to nominal In the Linux implementation of adc-keys (drivers/input/keyboard/adc-keys.c), `press-threshold-microvolt` is not really interpreted as a threshold, but rather as the "nominal voltage" of the button. When the voltage read from the ADC is closest to a button's `press-threshold-microvolt`, the button is considered pressed. This patch reconciles the behavior of button-adc with Linux's adc-keys such that device trees can be synchronized with minimal modifications. Signed-off-by: Peter Cai --- drivers/button/button-adc.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/button/button-adc.c b/drivers/button/button-adc.c index fd896c76f9d..9c24c960e6f 100644 --- a/drivers/button/button-adc.c +++ b/drivers/button/button-adc.c @@ -55,7 +55,7 @@ static int button_adc_of_to_plat(struct udevice *dev) struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev); struct button_adc_priv *priv = dev_get_priv(dev); struct ofnode_phandle_args args; - u32 threshold, up_threshold, t; + u32 down_threshold = 0, up_threshold, voltage, t; ofnode node; int ret; @@ -78,7 +78,7 @@ static int button_adc_of_to_plat(struct udevice *dev) return ret; ret = ofnode_read_u32(dev_ofnode(dev), "press-threshold-microvolt", - &threshold); + &voltage); if (ret) return ret; @@ -87,13 +87,24 @@ static int button_adc_of_to_plat(struct udevice *dev) if (ret) return ret; - if (t > threshold) + if (t > voltage && t < up_threshold) up_threshold = t; + else if (t < voltage && t > down_threshold) + down_threshold = t; } priv->channel = args.args[0]; - priv->min = threshold; - priv->max = up_threshold; + + /* + * Define the voltage range such that the button is only pressed + * when the voltage is closest to its own press-threshold-microvolt + */ + if (down_threshold == 0) + priv->min = 0; + else + priv->min = down_threshold + (voltage - down_threshold) / 2; + + priv->max = voltage + (up_threshold - voltage) / 2; return ret; } From 36fee2f7621eb2074be17bb0c4f8c950b0362c52 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Thu, 3 Feb 2022 15:14:47 +0100 Subject: [PATCH 03/18] common: fdt_support: add support for "partitions" subnode to fdt_fixup_mtdparts() Listing MTD partitions directly in the flash mode has been deprecated for a while for kernel Device Trees. Look for a node "partitions" in the found flash nodes and use it instead of the flash node itself for the partition list when it exists, so Device Trees following the current best practices can be fixed up. Signed-off-by: Matthias Schiffer Reviewed-by: Simon Glass --- common/fdt_support.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/common/fdt_support.c b/common/fdt_support.c index daa24d4c10b..ea18ea3f045 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -988,7 +988,7 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, { struct mtd_device *dev; int i, idx; - int noff; + int noff, parts; bool inited = false; for (i = 0; i < node_info_size; i++) { @@ -1014,7 +1014,12 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, dev = device_find(node_info[i].type, idx++); if (dev) { - if (fdt_node_set_part_info(blob, noff, dev)) + parts = fdt_subnode_offset(blob, noff, + "partitions"); + if (parts < 0) + parts = noff; + + if (fdt_node_set_part_info(blob, parts, dev)) return; /* return on error */ } } From eebcdb34d06104ed9ad2f089ad8a0b4f3b8ec89e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Thu, 3 Feb 2022 19:51:37 +0100 Subject: [PATCH 04/18] malloc_simple: Remove usage of unsupported %zx format string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace %zx by %lx and cast size_t to ulong. U-Boot currently prints garbage debug output: size=x, ptr=18, limit=18: 4002a000 With this change it prints correct debug data: size=18, ptr=18, limit=2000: 4002a000 Signed-off-by: Pali Rohár Reviewed-by: Simon Glass --- common/malloc_simple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 0267fb6bec8..67ee623850e 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -23,7 +23,7 @@ static void *alloc_simple(size_t bytes, int align) addr = ALIGN(gd->malloc_base + gd->malloc_ptr, align); new_ptr = addr + bytes - gd->malloc_base; - log_debug("size=%zx, ptr=%lx, limit=%lx: ", bytes, new_ptr, + log_debug("size=%lx, ptr=%lx, limit=%lx: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) { log_err("alloc space exhausted\n"); From 7ace56ae0321a0333d333df40e1e02aa17fa2dae Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Thu, 3 Feb 2022 21:43:50 +0100 Subject: [PATCH 05/18] test/py: Add test case for mkimage -o argument Stress the '-o algo_name' argument of mkimage by expanding the vboot test. Signed-off-by: Jan Kiszka Reviewed-by: Simon Glass [trini: Update scripts/pylint.base] --- scripts/pylint.base | 2 +- test/py/tests/test_vboot.py | 33 +++++++------- test/py/tests/vboot/sign-configs-algo-arg.its | 44 +++++++++++++++++++ test/py/tests/vboot/sign-images-algo-arg.its | 40 +++++++++++++++++ 4 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 test/py/tests/vboot/sign-configs-algo-arg.its create mode 100644 test/py/tests/vboot/sign-images-algo-arg.its diff --git a/scripts/pylint.base b/scripts/pylint.base index 7e25108b83c..e84ae52cbe9 100644 --- a/scripts/pylint.base +++ b/scripts/pylint.base @@ -66,7 +66,7 @@ test_tests_test_tpm2.py 8.51 test_tests_test_ums.py 6.32 test_tests_test_unknown_cmd.py 5.00 test_tests_test_ut.py 7.06 -test_tests_test_vboot.py 6.08 +test_tests_test_vboot.py 6.01 test_tests_vboot_evil.py 8.95 test_tests_vboot_forge.py 9.22 test_u_boot_console_base.py 7.08 diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index b080d482af9..ac8ed9f1145 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -35,18 +35,19 @@ import vboot_evil # Only run the full suite on a few combinations, since it doesn't add any more # test coverage. TESTDATA = [ - ['sha1-basic', 'sha1', '', None, False, True], - ['sha1-pad', 'sha1', '', '-E -p 0x10000', False, False], - ['sha1-pss', 'sha1', '-pss', None, False, False], - ['sha1-pss-pad', 'sha1', '-pss', '-E -p 0x10000', False, False], - ['sha256-basic', 'sha256', '', None, False, False], - ['sha256-pad', 'sha256', '', '-E -p 0x10000', False, False], - ['sha256-pss', 'sha256', '-pss', None, False, False], - ['sha256-pss-pad', 'sha256', '-pss', '-E -p 0x10000', False, False], - ['sha256-pss-required', 'sha256', '-pss', None, True, False], - ['sha256-pss-pad-required', 'sha256', '-pss', '-E -p 0x10000', True, True], - ['sha384-basic', 'sha384', '', None, False, False], - ['sha384-pad', 'sha384', '', '-E -p 0x10000', False, False], + ['sha1-basic', 'sha1', '', None, False, True, False], + ['sha1-pad', 'sha1', '', '-E -p 0x10000', False, False, False], + ['sha1-pss', 'sha1', '-pss', None, False, False, False], + ['sha1-pss-pad', 'sha1', '-pss', '-E -p 0x10000', False, False, False], + ['sha256-basic', 'sha256', '', None, False, False, False], + ['sha256-pad', 'sha256', '', '-E -p 0x10000', False, False, False], + ['sha256-pss', 'sha256', '-pss', None, False, False, False], + ['sha256-pss-pad', 'sha256', '-pss', '-E -p 0x10000', False, False, False], + ['sha256-pss-required', 'sha256', '-pss', None, True, False, False], + ['sha256-pss-pad-required', 'sha256', '-pss', '-E -p 0x10000', True, True, False], + ['sha384-basic', 'sha384', '', None, False, False, False], + ['sha384-pad', 'sha384', '', '-E -p 0x10000', False, False, False], + ['algo-arg', 'algo-arg', '', '-o sha256,rsa2048', False, False, True], ] @pytest.mark.boardspec('sandbox') @@ -55,10 +56,10 @@ TESTDATA = [ @pytest.mark.requiredtool('fdtget') @pytest.mark.requiredtool('fdtput') @pytest.mark.requiredtool('openssl') -@pytest.mark.parametrize("name,sha_algo,padding,sign_options,required,full_test", +@pytest.mark.parametrize("name,sha_algo,padding,sign_options,required,full_test,algo_arg", TESTDATA) def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required, - full_test): + full_test, algo_arg): """Test verified boot signing with mkimage and verification with 'bootm'. This works using sandbox only as it needs to update the device tree used @@ -219,7 +220,7 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required, # Build the FIT, but don't sign anything yet cons.log.action('%s: Test FIT with signed images' % sha_algo) make_fit('sign-images-%s%s.its' % (sha_algo, padding)) - run_bootm(sha_algo, 'unsigned images', 'dev-', True) + run_bootm(sha_algo, 'unsigned images', ' - OK' if algo_arg else 'dev-', True) # Sign images with our dev keys sign_fit(sha_algo, sign_options) @@ -230,7 +231,7 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required, cons.log.action('%s: Test FIT with signed configuration' % sha_algo) make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) - run_bootm(sha_algo, 'unsigned config', '%s+ OK' % sha_algo, True) + run_bootm(sha_algo, 'unsigned config', '%s+ OK' % ('sha256' if algo_arg else sha_algo), True) # Sign images with our dev keys sign_fit(sha_algo, sign_options) diff --git a/test/py/tests/vboot/sign-configs-algo-arg.its b/test/py/tests/vboot/sign-configs-algo-arg.its new file mode 100644 index 00000000000..3a5bb6d0f73 --- /dev/null +++ b/test/py/tests/vboot/sign-configs-algo-arg.its @@ -0,0 +1,44 @@ +/dts-v1/; + +/ { + description = "Chrome OS kernel image with one or more FDT blobs"; + #address-cells = <1>; + + images { + kernel { + data = /incbin/("test-kernel.bin"); + type = "kernel_noload"; + arch = "sandbox"; + os = "linux"; + compression = "none"; + load = <0x4>; + entry = <0x8>; + kernel-version = <1>; + hash-1 { + algo = "sha256"; + }; + }; + fdt-1 { + description = "snow"; + data = /incbin/("sandbox-kernel.dtb"); + type = "flat_dt"; + arch = "sandbox"; + compression = "none"; + fdt-version = <1>; + hash-1 { + algo = "sha256"; + }; + }; + }; + configurations { + default = "conf-1"; + conf-1 { + kernel = "kernel"; + fdt = "fdt-1"; + signature { + key-name-hint = "dev"; + sign-images = "fdt", "kernel"; + }; + }; + }; +}; diff --git a/test/py/tests/vboot/sign-images-algo-arg.its b/test/py/tests/vboot/sign-images-algo-arg.its new file mode 100644 index 00000000000..9144c8b5ad8 --- /dev/null +++ b/test/py/tests/vboot/sign-images-algo-arg.its @@ -0,0 +1,40 @@ +/dts-v1/; + +/ { + description = "Chrome OS kernel image with one or more FDT blobs"; + #address-cells = <1>; + + images { + kernel { + data = /incbin/("test-kernel.bin"); + type = "kernel_noload"; + arch = "sandbox"; + os = "linux"; + compression = "none"; + load = <0x4>; + entry = <0x8>; + kernel-version = <1>; + signature { + key-name-hint = "dev"; + }; + }; + fdt-1 { + description = "snow"; + data = /incbin/("sandbox-kernel.dtb"); + type = "flat_dt"; + arch = "sandbox"; + compression = "none"; + fdt-version = <1>; + signature { + key-name-hint = "dev"; + }; + }; + }; + configurations { + default = "conf-1"; + conf-1 { + kernel = "kernel"; + fdt = "fdt-1"; + }; + }; +}; From 63de067a1bcf57d66d6f68e6524f3e7fa8e5ea3d Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Fri, 4 Feb 2022 10:50:04 +0100 Subject: [PATCH 06/18] cmd: wrong printf() code in do_test_stackprot_fail() strlen() returns size_t. So we should use %zu to print it. This avoids incorrect output on 32bit systems. Fixes: 2fc62f299174 ("stackprot: Make our test a bit more complex") Signed-off-by: Heinrich Schuchardt --- cmd/stackprot_test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c index 1e26193e88b..f3470288fac 100644 --- a/cmd/stackprot_test.c +++ b/cmd/stackprot_test.c @@ -17,7 +17,8 @@ static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc, memset(a, 0xa5, 512); - printf("We have smashed our stack as this should not exceed 128: sizeof(a) = %ld\n", strlen(a)); + printf("We have smashed our stack as this should not exceed 128: sizeof(a) = %zd\n", + strlen(a)); return 0; } From 42db3738065d5adb41f7481d21eb2823c929b8e6 Mon Sep 17 00:00:00 2001 From: qthedev Date: Sat, 5 Feb 2022 10:25:16 +0000 Subject: [PATCH 07/18] Replace echo -n's used in environment processing with touch echo -n does not give the intended effect when invoked in macOS through /bin/sh, which is the shell make uses by default, see "https://stackoverflow.com/questions/11675070/makefile-echo-n-not-working" for a detailed explanation. In this case, it resulted in "-n" being written to env.txt and env.in even though they should be empty, which caused compilation to fail with "Your board uses a text-file environment, so must not define CONFIG_EXTRA_ENV_SETTINGS". This patch prevents the error by replacing echo -n's with touch, as they are used to create empty files in these cases. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 1ee7089c529..fa9bac41179 100644 --- a/Makefile +++ b/Makefile @@ -1843,7 +1843,7 @@ quiet_cmd_gen_envp = ENVP $@ -I$(srctree)/arch/$(ARCH)/include \ $< -o $@; \ else \ - echo -n >$@ ; \ + touch $@ ; \ fi include/generated/env.in: include/generated/env.txt FORCE $(call cmd,gen_envp) @@ -1860,7 +1860,7 @@ quiet_cmd_envc = ENVC $@ elif [ -n "$(ENV_SOURCE_FILE)" ]; then \ echo "Missing file $(ENV_FILE_CFG)"; \ else \ - echo -n >$@ ; \ + touch $@ ; \ fi include/generated/env.txt: $(wildcard $(ENV_FILE)) FORCE From 32a711dbdbec9aed6777d1059d115b1c83d36fca Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Sat, 5 Feb 2022 13:19:36 +0100 Subject: [PATCH 08/18] mkimage: Improve documentation of algo-name parameter Addresses the feedback provided on 5902a397d029 ("mkimage: Allow to specify the signature algorithm on the command line") which raced with the merge. Signed-off-by: Jan Kiszka Reviewed-by: Simon Glass --- doc/mkimage.1 | 2 +- tools/imagetool.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/mkimage.1 b/doc/mkimage.1 index 0734bd36a11..fc84cca066b 100644 --- a/doc/mkimage.1 +++ b/doc/mkimage.1 @@ -158,7 +158,7 @@ CONFIG_OF_CONTROL in U-Boot. .TP .BI "\-o [" "signing algorithm" "]" Specifies the algorithm to be used for signing a FIT image. The default is -taken from the target signature nodes 'algo' properties. +taken from the signature node's 'algo' property. .TP .BI "\-p [" "external position" "]" diff --git a/tools/imagetool.h b/tools/imagetool.h index a4105515d81..c3f80fc64e8 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -71,7 +71,9 @@ struct image_tool_params { const char *keydest; /* Destination .dtb for public key */ const char *keyfile; /* Filename of private or public key */ const char *comment; /* Comment to add to signature node */ - const char *algo_name; /* Algorithm name to use hashing/signing */ + /* Algorithm name to use for hashing/signing or NULL to use the one + * specified in the its */ + const char *algo_name; int require_keys; /* 1 to mark signing keys as 'required' */ int file_size; /* Total size of output file */ int orig_file_size; /* Original size for file before padding */ From d8ae90a8d47da2f22041bf9f6fd6d42a598f44ee Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Mon, 31 Jan 2022 11:52:20 +0900 Subject: [PATCH 09/18] DFU: Do not copy the entity name over the buffer size Use strlcpy() instead of strcpy() to prevent copying the entity name over the name buffer size. Signed-off-by: Masami Hiramatsu --- drivers/dfu/dfu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index af3975925a9..66c41b5e762 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -503,7 +503,7 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt, debug("%s: %s interface: %s dev: %s\n", __func__, s, interface, devstr); st = strsep(&s, " "); - strcpy(dfu->name, st); + strlcpy(dfu->name, st, DFU_NAME_SIZE); dfu->alt = alt; dfu->max_buf_size = 0; From 8db74c153b4e30edc5290da6c7330c63558678d0 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Mon, 31 Jan 2022 11:52:29 +0900 Subject: [PATCH 10/18] DFU: Accept redundant spaces and tabs in dfu_alt_info If dfu_alt_info has repeated spaces or tab (for indentation or readability), the dfu fails to parse it. For example, if dfu_alt_info="mtd nor1=image raw 100000 200000" (double spaces after "raw"), the image entity start address is '0' and the size '0x100000'. This is because the repeated space is not skipped. Use space and tab as a separater and apply skip_spaces() to skip redundant spaces and tabs. Signed-off-by: Masami Hiramatsu --- drivers/dfu/dfu.c | 6 ++++-- drivers/dfu/dfu_mmc.c | 11 ++++++++--- drivers/dfu/dfu_mtd.c | 8 ++++++-- drivers/dfu/dfu_nand.c | 11 ++++++++--- drivers/dfu/dfu_ram.c | 3 ++- drivers/dfu/dfu_sf.c | 12 +++++++++--- 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 66c41b5e762..18154774f9a 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -123,9 +123,10 @@ int dfu_config_interfaces(char *env) s = env; while (s) { ret = -EINVAL; - i = strsep(&s, " "); + i = strsep(&s, " \t"); if (!i) break; + s = skip_spaces(s); d = strsep(&s, "="); if (!d) break; @@ -502,8 +503,9 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt, char *st; debug("%s: %s interface: %s dev: %s\n", __func__, s, interface, devstr); - st = strsep(&s, " "); + st = strsep(&s, " \t"); strlcpy(dfu->name, st, DFU_NAME_SIZE); + s = skip_spaces(s); dfu->alt = alt; dfu->max_buf_size = 0; diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 3dab5a5f633..d747ede66c4 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -351,11 +351,12 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s) dfu->data.mmc.dev_num = dectoul(devstr, NULL); for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) { - *parg = strsep(&s, " "); + *parg = strsep(&s, " \t"); if (*parg == NULL) { pr_err("Invalid number of arguments.\n"); return -ENODEV; } + s = skip_spaces(s); } entity_type = argv[0]; @@ -390,9 +391,11 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s) * specifying the mmc HW defined partition number */ if (s) - if (!strcmp(strsep(&s, " "), "mmcpart")) + if (!strcmp(strsep(&s, " \t"), "mmcpart")) { + s = skip_spaces(s); dfu->data.mmc.hw_partition = simple_strtoul(s, NULL, 0); + } } else if (!strcmp(entity_type, "part")) { struct disk_partition partinfo; @@ -412,8 +415,10 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s) * specifying the mmc HW defined partition number */ if (s) - if (!strcmp(strsep(&s, " "), "offset")) + if (!strcmp(strsep(&s, " \t"), "offset")) { + s = skip_spaces(s); offset = simple_strtoul(s, NULL, 0); + } dfu->layout = DFU_RAW_ADDR; dfu->data.mmc.lba_start = partinfo.start + offset; diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c index cce9ce0845e..27c011f5379 100644 --- a/drivers/dfu/dfu_mtd.c +++ b/drivers/dfu/dfu_mtd.c @@ -12,6 +12,7 @@ #include #include #include +#include static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size) { @@ -285,11 +286,14 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s) dfu->data.mtd.info = mtd; dfu->max_buf_size = mtd->erasesize; - st = strsep(&s, " "); + st = strsep(&s, " \t"); + s = skip_spaces(s); if (!strcmp(st, "raw")) { dfu->layout = DFU_RAW_ADDR; dfu->data.mtd.start = hextoul(s, &s); - s++; + if (!isspace(*s)) + return -1; + s = skip_spaces(s); dfu->data.mtd.size = hextoul(s, &s); } else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) { char mtd_id[32]; diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c index e53b35e42b8..76761939ab5 100644 --- a/drivers/dfu/dfu_nand.c +++ b/drivers/dfu/dfu_nand.c @@ -201,11 +201,14 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s) dfu->data.nand.ubi = 0; dfu->dev_type = DFU_DEV_NAND; - st = strsep(&s, " "); + st = strsep(&s, " \t"); + s = skip_spaces(s); if (!strcmp(st, "raw")) { dfu->layout = DFU_RAW_ADDR; dfu->data.nand.start = hextoul(s, &s); - s++; + if (!isspace(*s)) + return -1; + s = skip_spaces(s); dfu->data.nand.size = hextoul(s, &s); } else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) { char mtd_id[32]; @@ -216,7 +219,9 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s) dfu->layout = DFU_RAW_ADDR; dev = dectoul(s, &s); - s++; + if (!isspace(*s)) + return -1; + s = skip_spaces(s); part = dectoul(s, &s); sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1); diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c index cc7e45ba335..361a3ff8af8 100644 --- a/drivers/dfu/dfu_ram.c +++ b/drivers/dfu/dfu_ram.c @@ -60,11 +60,12 @@ int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s) const char **parg = argv; for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) { - *parg = strsep(&s, " "); + *parg = strsep(&s, " \t"); if (*parg == NULL) { pr_err("Invalid number of arguments.\n"); return -ENODEV; } + s = skip_spaces(s); } dfu->dev_type = DFU_DEV_RAM; diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c index b72493ced86..993e951bc3b 100644 --- a/drivers/dfu/dfu_sf.c +++ b/drivers/dfu/dfu_sf.c @@ -13,6 +13,7 @@ #include #include #include +#include static int dfu_get_medium_size_sf(struct dfu_entity *dfu, u64 *size) { @@ -178,11 +179,14 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s) dfu->dev_type = DFU_DEV_SF; dfu->max_buf_size = dfu->data.sf.dev->sector_size; - st = strsep(&s, " "); + st = strsep(&s, " \t"); + s = skip_spaces(s); if (!strcmp(st, "raw")) { dfu->layout = DFU_RAW_ADDR; dfu->data.sf.start = hextoul(s, &s); - s++; + if (!isspace(*s)) + return -1; + s = skip_spaces(s); dfu->data.sf.size = hextoul(s, &s); } else if (CONFIG_IS_ENABLED(DFU_SF_PART) && (!strcmp(st, "part") || !strcmp(st, "partubi"))) { @@ -195,7 +199,9 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s) dfu->layout = DFU_RAW_ADDR; dev = dectoul(s, &s); - s++; + if (!isspace(*s)) + return -1; + s = skip_spaces(s); part = dectoul(s, &s); sprintf(mtd_id, "%s%d,%d", "nor", dev, part - 1); From 53b406369e9d0ba2da1df9b2488976c41acc6332 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Mon, 31 Jan 2022 11:52:37 +0900 Subject: [PATCH 11/18] DFU: Check the number of arguments and argument string strictly When parsing the dfu_alt_info, check the number of arguments and argument string strictly. If there is any garbage data (which is not able to be parsed correctly) in dfu_alt_info, that means something wrong and user may make a typo or mis- understanding about the syntax. Since the dfu_alt_info is used for updating the firmware, this mistake may lead to brick the hardware. Thus it should be checked strictly for making sure there is no mistake. Signed-off-by: Masami Hiramatsu --- drivers/dfu/dfu.c | 31 +++++++++++++++++------ drivers/dfu/dfu_mmc.c | 56 ++++++++++++++++++++++-------------------- drivers/dfu/dfu_mtd.c | 36 ++++++++++++++++----------- drivers/dfu/dfu_nand.c | 39 +++++++++++++++-------------- drivers/dfu/dfu_ram.c | 23 +++++++++-------- drivers/dfu/dfu_sf.c | 38 ++++++++++++++-------------- drivers/dfu/dfu_virt.c | 5 +++- include/dfu.h | 33 ++++++++++++++++--------- 8 files changed, 153 insertions(+), 108 deletions(-) diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 18154774f9a..516dda61796 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -500,12 +500,29 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt, char *interface, char *devstr) { + char *argv[DFU_MAX_ENTITY_ARGS]; + int argc; char *st; debug("%s: %s interface: %s dev: %s\n", __func__, s, interface, devstr); st = strsep(&s, " \t"); strlcpy(dfu->name, st, DFU_NAME_SIZE); - s = skip_spaces(s); + + /* Parse arguments */ + for (argc = 0; s && argc < DFU_MAX_ENTITY_ARGS; argc++) { + s = skip_spaces(s); + if (!*s) + break; + argv[argc] = strsep(&s, " \t"); + } + + if (argc == DFU_MAX_ENTITY_ARGS && s) { + s = skip_spaces(s); + if (*s) { + log_err("Too many arguments for %s\n", dfu->name); + return -EINVAL; + } + } dfu->alt = alt; dfu->max_buf_size = 0; @@ -513,22 +530,22 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt, /* Specific for mmc device */ if (strcmp(interface, "mmc") == 0) { - if (dfu_fill_entity_mmc(dfu, devstr, s)) + if (dfu_fill_entity_mmc(dfu, devstr, argv, argc)) return -1; } else if (strcmp(interface, "mtd") == 0) { - if (dfu_fill_entity_mtd(dfu, devstr, s)) + if (dfu_fill_entity_mtd(dfu, devstr, argv, argc)) return -1; } else if (strcmp(interface, "nand") == 0) { - if (dfu_fill_entity_nand(dfu, devstr, s)) + if (dfu_fill_entity_nand(dfu, devstr, argv, argc)) return -1; } else if (strcmp(interface, "ram") == 0) { - if (dfu_fill_entity_ram(dfu, devstr, s)) + if (dfu_fill_entity_ram(dfu, devstr, argv, argc)) return -1; } else if (strcmp(interface, "sf") == 0) { - if (dfu_fill_entity_sf(dfu, devstr, s)) + if (dfu_fill_entity_sf(dfu, devstr, argv, argc)) return -1; } else if (strcmp(interface, "virt") == 0) { - if (dfu_fill_entity_virt(dfu, devstr, s)) + if (dfu_fill_entity_virt(dfu, devstr, argv, argc)) return -1; } else { printf("%s: Device %s not (yet) supported!\n", diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index d747ede66c4..a91da972d46 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -337,35 +337,34 @@ void dfu_free_entity_mmc(struct dfu_entity *dfu) * 4th (optional): * mmcpart (access to HW eMMC partitions) */ -int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s) +int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char **argv, int argc) { const char *entity_type; ssize_t second_arg; size_t third_arg; - struct mmc *mmc; + char *s; - const char *argv[3]; - const char **parg = argv; - - dfu->data.mmc.dev_num = dectoul(devstr, NULL); - - for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) { - *parg = strsep(&s, " \t"); - if (*parg == NULL) { - pr_err("Invalid number of arguments.\n"); - return -ENODEV; - } - s = skip_spaces(s); + if (argc < 3) { + pr_err("The number of parameters are not enough.\n"); + return -EINVAL; } + dfu->data.mmc.dev_num = dectoul(devstr, &s); + if (*s) + return -EINVAL; + entity_type = argv[0]; /* * Base 0 means we'll accept (prefixed with 0x or 0) base 16, 8, * with default 10. */ - second_arg = simple_strtol(argv[1], NULL, 0); - third_arg = simple_strtoul(argv[2], NULL, 0); + second_arg = simple_strtol(argv[1], &s, 0); + if (*s) + return -EINVAL; + third_arg = simple_strtoul(argv[2], &s, 0); + if (*s) + return -EINVAL; mmc = find_mmc_device(dfu->data.mmc.dev_num); if (mmc == NULL) { @@ -390,12 +389,14 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s) * Check for an extra entry at dfu_alt_info env variable * specifying the mmc HW defined partition number */ - if (s) - if (!strcmp(strsep(&s, " \t"), "mmcpart")) { - s = skip_spaces(s); - dfu->data.mmc.hw_partition = - simple_strtoul(s, NULL, 0); + if (argc > 3) { + if (argc != 5 || strcmp(argv[3], "mmcpart")) { + pr_err("DFU mmc raw accept 'mmcpart ' option.\n"); + return -EINVAL; } + dfu->data.mmc.hw_partition = + simple_strtoul(argv[4], NULL, 0); + } } else if (!strcmp(entity_type, "part")) { struct disk_partition partinfo; @@ -414,15 +415,18 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s) * Check for an extra entry at dfu_alt_info env variable * specifying the mmc HW defined partition number */ - if (s) - if (!strcmp(strsep(&s, " \t"), "offset")) { - s = skip_spaces(s); - offset = simple_strtoul(s, NULL, 0); + if (argc > 3) { + if (argc != 5 || strcmp(argv[3], "offset")) { + pr_err("DFU mmc raw accept 'mmcpart ' option.\n"); + return -EINVAL; } + dfu->data.mmc.hw_partition = + simple_strtoul(argv[4], NULL, 0); + } dfu->layout = DFU_RAW_ADDR; dfu->data.mmc.lba_start = partinfo.start + offset; - dfu->data.mmc.lba_size = partinfo.size-offset; + dfu->data.mmc.lba_size = partinfo.size - offset; dfu->data.mmc.lba_blk_size = partinfo.blksz; } else if (!strcmp(entity_type, "fat")) { dfu->layout = DFU_FS_FAT; diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c index 27c011f5379..c7075f12eca 100644 --- a/drivers/dfu/dfu_mtd.c +++ b/drivers/dfu/dfu_mtd.c @@ -271,9 +271,9 @@ static unsigned int dfu_polltimeout_mtd(struct dfu_entity *dfu) return DFU_DEFAULT_POLL_TIMEOUT; } -int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s) +int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char **argv, int argc) { - char *st; + char *s; struct mtd_info *mtd; int ret, part; @@ -285,25 +285,33 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s) dfu->dev_type = DFU_DEV_MTD; dfu->data.mtd.info = mtd; dfu->max_buf_size = mtd->erasesize; + if (argc < 1) + return -EINVAL; - st = strsep(&s, " \t"); - s = skip_spaces(s); - if (!strcmp(st, "raw")) { + if (!strcmp(argv[0], "raw")) { + if (argc != 3) + return -EINVAL; dfu->layout = DFU_RAW_ADDR; - dfu->data.mtd.start = hextoul(s, &s); - if (!isspace(*s)) - return -1; - s = skip_spaces(s); - dfu->data.mtd.size = hextoul(s, &s); - } else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) { + dfu->data.mtd.start = hextoul(argv[1], &s); + if (*s) + return -EINVAL; + dfu->data.mtd.size = hextoul(argv[2], &s); + if (*s) + return -EINVAL; + } else if ((!strcmp(argv[0], "part")) || (!strcmp(argv[0], "partubi"))) { char mtd_id[32]; struct mtd_device *mtd_dev; u8 part_num; struct part_info *pi; + if (argc != 2) + return -EINVAL; + dfu->layout = DFU_RAW_ADDR; - part = dectoul(s, &s); + part = dectoul(argv[1], &s); + if (*s) + return -EINVAL; sprintf(mtd_id, "%s,%d", devstr, part - 1); printf("using id '%s'\n", mtd_id); @@ -318,10 +326,10 @@ int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s) dfu->data.mtd.start = pi->offset; dfu->data.mtd.size = pi->size; - if (!strcmp(st, "partubi")) + if (!strcmp(argv[0], "partubi")) dfu->data.mtd.ubi = 1; } else { - printf("%s: Memory layout (%s) not supported!\n", __func__, st); + printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]); return -1; } diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c index 76761939ab5..08e8cf5cdb3 100644 --- a/drivers/dfu/dfu_nand.c +++ b/drivers/dfu/dfu_nand.c @@ -194,23 +194,25 @@ unsigned int dfu_polltimeout_nand(struct dfu_entity *dfu) return DFU_DEFAULT_POLL_TIMEOUT; } -int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s) +int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char **argv, int argc) { - char *st; + char *s; int ret, dev, part; dfu->data.nand.ubi = 0; dfu->dev_type = DFU_DEV_NAND; - st = strsep(&s, " \t"); - s = skip_spaces(s); - if (!strcmp(st, "raw")) { + if (argc != 3) + return -EINVAL; + + if (!strcmp(argv[0], "raw")) { dfu->layout = DFU_RAW_ADDR; - dfu->data.nand.start = hextoul(s, &s); - if (!isspace(*s)) - return -1; - s = skip_spaces(s); - dfu->data.nand.size = hextoul(s, &s); - } else if ((!strcmp(st, "part")) || (!strcmp(st, "partubi"))) { + dfu->data.nand.start = hextoul(argv[1], &s); + if (*s) + return -EINVAL; + dfu->data.nand.size = hextoul(argv[2], &s); + if (*s) + return -EINVAL; + } else if ((!strcmp(argv[0], "part")) || (!strcmp(argv[0], "partubi"))) { char mtd_id[32]; struct mtd_device *mtd_dev; u8 part_num; @@ -218,11 +220,12 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s) dfu->layout = DFU_RAW_ADDR; - dev = dectoul(s, &s); - if (!isspace(*s)) - return -1; - s = skip_spaces(s); - part = dectoul(s, &s); + dev = dectoul(argv[1], &s); + if (*s) + return -EINVAL; + part = dectoul(argv[2], &s); + if (*s) + return -EINVAL; sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1); debug("using id '%s'\n", mtd_id); @@ -237,10 +240,10 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s) dfu->data.nand.start = pi->offset; dfu->data.nand.size = pi->size; - if (!strcmp(st, "partubi")) + if (!strcmp(argv[0], "partubi")) dfu->data.nand.ubi = 1; } else { - printf("%s: Memory layout (%s) not supported!\n", __func__, st); + printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]); return -1; } diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c index 361a3ff8af8..9d10303164e 100644 --- a/drivers/dfu/dfu_ram.c +++ b/drivers/dfu/dfu_ram.c @@ -54,18 +54,13 @@ static int dfu_read_medium_ram(struct dfu_entity *dfu, u64 offset, return dfu_transfer_medium_ram(DFU_OP_READ, dfu, offset, buf, len); } -int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s) +int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char **argv, int argc) { - const char *argv[3]; - const char **parg = argv; + char *s; - for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) { - *parg = strsep(&s, " \t"); - if (*parg == NULL) { - pr_err("Invalid number of arguments.\n"); - return -ENODEV; - } - s = skip_spaces(s); + if (argc != 3) { + pr_err("Invalid number of arguments.\n"); + return -EINVAL; } dfu->dev_type = DFU_DEV_RAM; @@ -75,8 +70,12 @@ int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s) } dfu->layout = DFU_RAM_ADDR; - dfu->data.ram.start = hextoul(argv[1], NULL); - dfu->data.ram.size = hextoul(argv[2], NULL); + dfu->data.ram.start = hextoul(argv[1], &s); + if (*s) + return -EINVAL; + dfu->data.ram.size = hextoul(argv[2], &s); + if (*s) + return -EINVAL; dfu->write_medium = dfu_write_medium_ram; dfu->get_medium_size = dfu_get_medium_size_ram; diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c index 993e951bc3b..25a9c818503 100644 --- a/drivers/dfu/dfu_sf.c +++ b/drivers/dfu/dfu_sf.c @@ -166,9 +166,9 @@ static struct spi_flash *parse_dev(char *devstr) return dev; } -int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s) +int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char **argv, int argc) { - char *st; + char *s; char *devstr_bkup = strdup(devstr); dfu->data.sf.dev = parse_dev(devstr_bkup); @@ -179,17 +179,18 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s) dfu->dev_type = DFU_DEV_SF; dfu->max_buf_size = dfu->data.sf.dev->sector_size; - st = strsep(&s, " \t"); - s = skip_spaces(s); - if (!strcmp(st, "raw")) { + if (argc != 3) + return -EINVAL; + if (!strcmp(argv[0], "raw")) { dfu->layout = DFU_RAW_ADDR; - dfu->data.sf.start = hextoul(s, &s); - if (!isspace(*s)) - return -1; - s = skip_spaces(s); - dfu->data.sf.size = hextoul(s, &s); + dfu->data.sf.start = hextoul(argv[1], &s); + if (*s) + return -EINVAL; + dfu->data.sf.size = hextoul(argv[2], &s); + if (*s) + return -EINVAL; } else if (CONFIG_IS_ENABLED(DFU_SF_PART) && - (!strcmp(st, "part") || !strcmp(st, "partubi"))) { + (!strcmp(argv[0], "part") || !strcmp(argv[0], "partubi"))) { char mtd_id[32]; struct mtd_device *mtd_dev; u8 part_num; @@ -198,11 +199,12 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s) dfu->layout = DFU_RAW_ADDR; - dev = dectoul(s, &s); - if (!isspace(*s)) - return -1; - s = skip_spaces(s); - part = dectoul(s, &s); + dev = dectoul(argv[1], &s); + if (*s) + return -EINVAL; + part = dectoul(argv[2], &s); + if (*s) + return -EINVAL; sprintf(mtd_id, "%s%d,%d", "nor", dev, part - 1); printf("using id '%s'\n", mtd_id); @@ -216,10 +218,10 @@ int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s) } dfu->data.sf.start = pi->offset; dfu->data.sf.size = pi->size; - if (!strcmp(st, "partubi")) + if (!strcmp(argv[0], "partubi")) dfu->data.sf.ubi = 1; } else { - printf("%s: Memory layout (%s) not supported!\n", __func__, st); + printf("%s: Memory layout (%s) not supported!\n", __func__, argv[0]); spi_flash_free(dfu->data.sf.dev); return -1; } diff --git a/drivers/dfu/dfu_virt.c b/drivers/dfu/dfu_virt.c index 80c99cb06e3..29f7a08f672 100644 --- a/drivers/dfu/dfu_virt.c +++ b/drivers/dfu/dfu_virt.c @@ -32,10 +32,13 @@ int __weak dfu_read_medium_virt(struct dfu_entity *dfu, u64 offset, return 0; } -int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char *s) +int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char **argv, int argc) { debug("%s: devstr = %s\n", __func__, devstr); + if (argc != 0) + return -EINVAL; + dfu->dev_type = DFU_DEV_VIRT; dfu->layout = DFU_RAW_ADDR; dfu->data.virt.dev_num = dectoul(devstr, NULL); diff --git a/include/dfu.h b/include/dfu.h index f6868982df7..dcb9cd9d799 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -432,11 +432,15 @@ static inline void dfu_set_defer_flush(struct dfu_entity *dfu) int dfu_write_from_mem_addr(struct dfu_entity *dfu, void *buf, int size); /* Device specific */ +/* Each entity has 5 arguments in maximum. */ +#define DFU_MAX_ENTITY_ARGS 5 + #if CONFIG_IS_ENABLED(DFU_MMC) -extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s); +extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, + char **argv, int argc); #else static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, - char *s) + char **argv, int argc) { puts("MMC support not available!\n"); return -1; @@ -444,10 +448,11 @@ static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, #endif #if CONFIG_IS_ENABLED(DFU_NAND) -extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, char *s); +extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, + char **argv, int argc); #else static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, - char *s) + char **argv, int argc) { puts("NAND support not available!\n"); return -1; @@ -455,10 +460,11 @@ static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *devstr, #endif #if CONFIG_IS_ENABLED(DFU_RAM) -extern int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, char *s); +extern int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, + char **argv, int argc); #else static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, - char *s) + char **argv, int argc) { puts("RAM support not available!\n"); return -1; @@ -466,10 +472,11 @@ static inline int dfu_fill_entity_ram(struct dfu_entity *dfu, char *devstr, #endif #if CONFIG_IS_ENABLED(DFU_SF) -extern int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, char *s); +extern int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, + char **argv, int argc); #else static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, - char *s) + char **argv, int argc) { puts("SF support not available!\n"); return -1; @@ -477,10 +484,11 @@ static inline int dfu_fill_entity_sf(struct dfu_entity *dfu, char *devstr, #endif #if CONFIG_IS_ENABLED(DFU_MTD) -int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, char *s); +extern int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, + char **argv, int argc); #else static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, - char *s) + char **argv, int argc) { puts("MTD support not available!\n"); return -1; @@ -488,7 +496,8 @@ static inline int dfu_fill_entity_mtd(struct dfu_entity *dfu, char *devstr, #endif #ifdef CONFIG_DFU_VIRT -int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, char *s); +int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, + char **argv, int argc); int dfu_write_medium_virt(struct dfu_entity *dfu, u64 offset, void *buf, long *len); int dfu_get_medium_size_virt(struct dfu_entity *dfu, u64 *size); @@ -496,7 +505,7 @@ int dfu_read_medium_virt(struct dfu_entity *dfu, u64 offset, void *buf, long *len); #else static inline int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, - char *s) + char **argv, int argc) { puts("VIRT support not available!\n"); return -1; From c25a838c9ca2416fe601b656b3b5adb64feb925c Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Mon, 31 Jan 2022 11:52:46 +0900 Subject: [PATCH 12/18] doc: usage: DFU: Fix dfu_alt_info document Fix some typo and wrong information about dfu_alt_info. - Add the parameter format, decimal only or hexadecimal. - Use same parameter name for the same kind of parameters. (e.g. dev -> dev_id) Signed-off-by: Masami Hiramatsu --- doc/usage/dfu.rst | 57 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/doc/usage/dfu.rst b/doc/usage/dfu.rst index 11c88072b89..ed47ff561e3 100644 --- a/doc/usage/dfu.rst +++ b/doc/usage/dfu.rst @@ -113,9 +113,9 @@ mmc each element in *dfu_alt_info* being * raw [mmcpart ] raw access to mmc device - * part [mmcpart ] raw access to partition - * fat [mmcpart ] file in FAT partition - * ext4 [mmcpart ] file in EXT4 partition + * part [offset ] raw access to partition + * fat file in FAT partition + * ext4 file in EXT4 partition * skip 0 0 ignore flashed data * script 0 0 execute commands in shell @@ -169,14 +169,20 @@ nand each element in *dfu_alt_info* being either of - * raw raw access to mmc device - * part raw acces to partition - * partubi raw acces to ubi partition + * raw raw access to nand device + * part raw access to partition + * partubi raw access to ubi partition with - partid - is the MTD partition index + offset + is the offset in the nand device (hexadecimal without "0x") + size + is the size of the access area (hexadecimal without "0x") + dev_id + is the NAND device index (decimal only) + part_id + is the NAND partition index (decimal only) ram raw access to ram:: @@ -190,6 +196,13 @@ ram ram raw access to ram + with + + offset + is the offset in the ram device (hexadecimal without "0x") + size + is the size of the access area (hexadecimal without "0x") + sf serial flash : NOR:: @@ -198,13 +211,19 @@ sf each element in *dfu_alt_info* being either of: * raw raw access to sf device - * part raw acces to partition - * partubi raw acces to ubi partition + * part raw acces to partition + * partubi raw acces to ubi partition with - partid - is the MTD partition index + offset + is the offset in the sf device (hexadecimal without "0x") + size + is the size of the access area (hexadecimal without "0x") + dev_id + is the sf device index (the device is "nor") (deximal only) + part_id + is the MTD partition index (decimal only) mtd all MTD device: NAND, SPI-NOR, SPI-NAND,...:: @@ -219,14 +238,18 @@ mtd each element in *dfu_alt_info* being either of: - * raw forraw access to mtd device - * part for raw acces to partition - * partubi for raw acces to ubi partition + * raw for raw access to mtd device + * part for raw access to partition + * partubi for raw access to ubi partition with - partid - is the MTD partition index + offset + is the offset in the mtd device (hexadecimal without "0x") + size + is the size of the access area (hexadecimal without "0x") + part_id + is the MTD partition index (decimal only) virt virtual flash back end for DFU From e9b0fd839bce97f5ef0380fd618cab28a7109aba Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Mon, 31 Jan 2022 11:52:54 +0900 Subject: [PATCH 13/18] cmd/dfu: Enable 'dfu list' command without DFU_OVER_USB Since dfu is not only used for USB, and some platform only supports DFU_OVER_TFTP or EFI capsule update, dfu_alt_info is defined on such platforms too. For such platform, 'dfu list' command is useful to check how the current dfu_alt_info setting is parsed. Signed-off-by: Masami Hiramatsu --- cmd/dfu.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cmd/dfu.c b/cmd/dfu.c index 4a288f74c2c..d7bfb535dc6 100644 --- a/cmd/dfu.c +++ b/cmd/dfu.c @@ -28,7 +28,6 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) #ifdef CONFIG_DFU_OVER_USB char *usb_controller = argv[1]; #endif -#if defined(CONFIG_DFU_OVER_USB) || defined(CONFIG_DFU_OVER_TFTP) char *interface = NULL; char *devstring = NULL; #if defined(CONFIG_DFU_TIMEOUT) || defined(CONFIG_DFU_OVER_TFTP) @@ -42,7 +41,6 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) #if defined(CONFIG_DFU_TIMEOUT) || defined(CONFIG_DFU_OVER_TFTP) if (argc == 5 || argc == 3) value = simple_strtoul(argv[argc - 1], NULL, 0); -#endif #endif int ret = 0; @@ -50,7 +48,6 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (!strcmp(argv[1], "tftp")) return update_tftp(value, interface, devstring); #endif -#ifdef CONFIG_DFU_OVER_USB ret = dfu_init_env_entities(interface, devstring); if (ret) goto done; @@ -65,6 +62,7 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) goto done; } +#ifdef CONFIG_DFU_OVER_USB int controller_index = simple_strtoul(usb_controller, NULL, 0); bool retry = false; do { @@ -79,9 +77,9 @@ static int do_dfu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } } while (retry); +#endif done: dfu_free_entities(); -#endif return ret; } @@ -100,8 +98,8 @@ U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu, #ifdef CONFIG_DFU_TIMEOUT " [] - specify inactivity timeout in seconds\n" #endif - " [list] - list available alt settings\n" #endif + " [list] - list available alt settings\n" #ifdef CONFIG_DFU_OVER_TFTP #ifdef CONFIG_DFU_OVER_USB "dfu " From aafb31fc953aac0af89e44a1e4a841803141768d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 29 Jan 2022 14:30:52 -0700 Subject: [PATCH 14/18] acpi: Move acpi_write_tables() to a generic header This function is used by both x86 and sandbox. Put it in a common header file. Signed-off-by: Simon Glass --- arch/sandbox/include/asm/acpi_table.h | 2 +- arch/x86/include/asm/acpi_table.h | 2 -- include/acpi/acpi_table.h | 10 ++++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/arch/sandbox/include/asm/acpi_table.h b/arch/sandbox/include/asm/acpi_table.h index ae17f6c5197..cb10eb542b2 100644 --- a/arch/sandbox/include/asm/acpi_table.h +++ b/arch/sandbox/include/asm/acpi_table.h @@ -6,6 +6,6 @@ #ifndef __ASM_ACPI_TABLE_H__ #define __ASM_ACPI_TABLE_H__ -ulong write_acpi_tables(ulong start); +/* Empty for now, this file is required by acpi/acpi_table.h */ #endif /* __ASM_ACPI_TABLE_H__ */ diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h index 39547de0d4d..226753b65d6 100644 --- a/arch/x86/include/asm/acpi_table.h +++ b/arch/x86/include/asm/acpi_table.h @@ -64,8 +64,6 @@ int acpi_write_dbg2_pci_uart(struct acpi_ctx *ctx, struct udevice *dev, */ int acpi_create_gnvs(struct acpi_global_nvs *gnvs); -ulong write_acpi_tables(ulong start); - /** * acpi_get_rsdp_addr() - get ACPI RSDP table address * diff --git a/include/acpi/acpi_table.h b/include/acpi/acpi_table.h index c98c874fe40..4030d25c66a 100644 --- a/include/acpi/acpi_table.h +++ b/include/acpi/acpi_table.h @@ -913,6 +913,16 @@ void acpi_fill_header(struct acpi_table_header *header, char *signature); */ int acpi_fill_csrt(struct acpi_ctx *ctx); +/** + * write_acpi_tables() - Write out the ACPI tables + * + * This writes all ACPI tables to the given address + * + * @start: Start address for the tables + * @return address of end of tables, where the next tables can be written + */ +ulong write_acpi_tables(ulong start); + #endif /* !__ACPI__*/ #include From f178f7c9550c4fd9c644f79a1eb2dafa5bcdce25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Wed, 12 Jan 2022 12:47:05 +0100 Subject: [PATCH 15/18] fw_env: make flash_io() take buffer as an argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's usually easier to understand code & follow it if all arguments are passed explicitly. Many coding styles also discourage using global variables. Behaviour of flash_io() was a bit unintuitive as it was writing to a buffer referenced in a global struct. That required developers to remember how it works and sometimes required hacking "environment" global struct variable to read data into a proper buffer. Signed-off-by: Rafał Miłecki --- tools/env/fw_env.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 3da75be7830..a738b910c29 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -346,7 +346,7 @@ static int ubi_write(int fd, const void *buf, size_t count) return 0; } -static int flash_io(int mode); +static int flash_io(int mode, void *buf, size_t count); static int parse_config(struct env_opts *opts); #if defined(CONFIG_FILE) @@ -516,7 +516,7 @@ int fw_env_flush(struct env_opts *opts) *environment.crc = crc32(0, (uint8_t *) environment.data, ENV_SIZE); /* write environment back to flash */ - if (flash_io(O_RDWR)) { + if (flash_io(O_RDWR, environment.image, CUR_ENVSIZE)) { fprintf(stderr, "Error: can't write fw_env to flash\n"); return -1; } @@ -1185,7 +1185,8 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset) return rc; } -static int flash_write(int fd_current, int fd_target, int dev_target) +static int flash_write(int fd_current, int fd_target, int dev_target, void *buf, + size_t count) { int rc; @@ -1212,11 +1213,10 @@ static int flash_write(int fd_current, int fd_target, int dev_target) if (IS_UBI(dev_target)) { if (ubi_update_start(fd_target, CUR_ENVSIZE) < 0) return -1; - return ubi_write(fd_target, environment.image, CUR_ENVSIZE); + return ubi_write(fd_target, buf, count); } - rc = flash_write_buf(dev_target, fd_target, environment.image, - CUR_ENVSIZE); + rc = flash_write_buf(dev_target, fd_target, buf, count); if (rc < 0) return rc; @@ -1235,17 +1235,17 @@ static int flash_write(int fd_current, int fd_target, int dev_target) return 0; } -static int flash_read(int fd) +static int flash_read(int fd, void *buf, size_t count) { int rc; if (IS_UBI(dev_current)) { DEVTYPE(dev_current) = MTD_ABSENT; - return ubi_read(fd, environment.image, CUR_ENVSIZE); + return ubi_read(fd, buf, count); } - rc = flash_read_buf(dev_current, fd, environment.image, CUR_ENVSIZE, + rc = flash_read_buf(dev_current, fd, buf, count, DEVOFFSET(dev_current)); if (rc != CUR_ENVSIZE) return -1; @@ -1291,7 +1291,7 @@ err: return rc; } -static int flash_io_write(int fd_current) +static int flash_io_write(int fd_current, void *buf, size_t count) { int fd_target = -1, rc, dev_target; const char *dname, *target_temp = NULL; @@ -1322,7 +1322,7 @@ static int flash_io_write(int fd_current) fd_target = fd_current; } - rc = flash_write(fd_current, fd_target, dev_target); + rc = flash_write(fd_current, fd_target, dev_target, buf, count); if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) { fprintf(stderr, @@ -1377,7 +1377,7 @@ static int flash_io_write(int fd_current) return rc; } -static int flash_io(int mode) +static int flash_io(int mode, void *buf, size_t count) { int fd_current, rc; @@ -1391,9 +1391,9 @@ static int flash_io(int mode) } if (mode == O_RDWR) { - rc = flash_io_write(fd_current); + rc = flash_io_write(fd_current, buf, count); } else { - rc = flash_read(fd_current); + rc = flash_read(fd_current, buf, count); } if (close(fd_current)) { @@ -1455,7 +1455,7 @@ int fw_env_open(struct env_opts *opts) } dev_current = 0; - if (flash_io(O_RDONLY)) { + if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) { ret = -EIO; goto open_cleanup; } @@ -1490,7 +1490,7 @@ int fw_env_open(struct env_opts *opts) * other pointers in environment still point inside addr0 */ environment.image = addr1; - if (flash_io(O_RDONLY)) { + if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) { ret = -EIO; goto open_cleanup; } From 07c79dd5fdeaefb39c9e7a97f3b66de63109a18d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Wed, 12 Jan 2022 12:47:06 +0100 Subject: [PATCH 16/18] fw_env: simplify logic & code paths in the fw_env_open() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Environment variables can be stored in two formats: 1. Single entry with header containing CRC32 2. Two entries with extra flags field in each entry header For that reason fw_env_open() has two main code paths and there are pointers for CRC32/flags/data. Previous implementation was a bit hard to follow: 1. It was checking for used format twice (in reversed order each time) 2. It was setting "environment" global struct fields to some temporary values that required extra comments explaining it This change simplifies that code: 1. It introduces two clear code paths 2. It sets "environment" global struct fields values only once it really knows them To be fair there are *two* crc32() calls now and an extra pointer variable but that should be cheap enough and worth it. Signed-off-by: Rafał Miłecki --- tools/env/fw_env.c | 77 +++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 46 deletions(-) diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index a738b910c29..31afef6f3b1 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1421,9 +1421,6 @@ int fw_env_open(struct env_opts *opts) int ret; - struct env_image_single *single; - struct env_image_redundant *redundant; - if (!opts) opts = &default_opts; @@ -1439,40 +1436,37 @@ int fw_env_open(struct env_opts *opts) goto open_cleanup; } - /* read environment from FLASH to local buffer */ - environment.image = addr0; - - if (have_redund_env) { - redundant = addr0; - environment.crc = &redundant->crc; - environment.flags = &redundant->flags; - environment.data = redundant->data; - } else { - single = addr0; - environment.crc = &single->crc; - environment.flags = NULL; - environment.data = single->data; - } - dev_current = 0; - if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) { + if (flash_io(O_RDONLY, addr0, CUR_ENVSIZE)) { ret = -EIO; goto open_cleanup; } - crc0 = crc32(0, (uint8_t *)environment.data, ENV_SIZE); - - crc0_ok = (crc0 == *environment.crc); if (!have_redund_env) { + struct env_image_single *single = addr0; + + crc0 = crc32(0, (uint8_t *)single->data, ENV_SIZE); + crc0_ok = (crc0 == single->crc); if (!crc0_ok) { fprintf(stderr, "Warning: Bad CRC, using default environment\n"); - memcpy(environment.data, default_environment, + memcpy(single->data, default_environment, sizeof(default_environment)); environment.dirty = 1; } + + environment.image = addr0; + environment.crc = &single->crc; + environment.flags = NULL; + environment.data = single->data; } else { - flag0 = *environment.flags; + struct env_image_redundant *redundant0 = addr0; + struct env_image_redundant *redundant1; + + crc0 = crc32(0, (uint8_t *)redundant0->data, ENV_SIZE); + crc0_ok = (crc0 == redundant0->crc); + + flag0 = redundant0->flags; dev_current = 1; addr1 = calloc(1, CUR_ENVSIZE); @@ -1483,14 +1477,9 @@ int fw_env_open(struct env_opts *opts) ret = -ENOMEM; goto open_cleanup; } - redundant = addr1; + redundant1 = addr1; - /* - * have to set environment.image for flash_read(), careful - - * other pointers in environment still point inside addr0 - */ - environment.image = addr1; - if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) { + if (flash_io(O_RDONLY, addr1, CUR_ENVSIZE)) { ret = -EIO; goto open_cleanup; } @@ -1518,18 +1507,12 @@ int fw_env_open(struct env_opts *opts) goto open_cleanup; } - crc1 = crc32(0, (uint8_t *)redundant->data, ENV_SIZE); + crc1 = crc32(0, (uint8_t *)redundant1->data, ENV_SIZE); - crc1_ok = (crc1 == redundant->crc); - flag1 = redundant->flags; + crc1_ok = (crc1 == redundant1->crc); + flag1 = redundant1->flags; - /* - * environment.data still points to ((struct - * env_image_redundant *)addr0)->data. If the two - * environments differ, or one has bad crc, force a - * write-out by marking the environment dirty. - */ - if (memcmp(environment.data, redundant->data, ENV_SIZE) || + if (memcmp(redundant0->data, redundant1->data, ENV_SIZE) || !crc0_ok || !crc1_ok) environment.dirty = 1; @@ -1540,7 +1523,7 @@ int fw_env_open(struct env_opts *opts) } else if (!crc0_ok && !crc1_ok) { fprintf(stderr, "Warning: Bad CRC, using default environment\n"); - memcpy(environment.data, default_environment, + memcpy(redundant0->data, default_environment, sizeof(default_environment)); environment.dirty = 1; dev_current = 0; @@ -1586,13 +1569,15 @@ int fw_env_open(struct env_opts *opts) */ if (dev_current) { environment.image = addr1; - environment.crc = &redundant->crc; - environment.flags = &redundant->flags; - environment.data = redundant->data; + environment.crc = &redundant1->crc; + environment.flags = &redundant1->flags; + environment.data = redundant1->data; free(addr0); } else { environment.image = addr0; - /* Other pointers are already set */ + environment.crc = &redundant0->crc; + environment.flags = &redundant0->flags; + environment.data = redundant0->data; free(addr1); } #ifdef DEBUG From 3970c82a60857b72afcb676697caf9c979dab946 Mon Sep 17 00:00:00 2001 From: Adam Ford Date: Sat, 29 Jan 2022 07:27:47 -0600 Subject: [PATCH 17/18] phy: nop-phy: Enable reset-gpios support Some usb-nop-xceiv devices use a gpio take them out of reset. Add a reset function to put them into that state. This is similar to how Linux handles the usb-nop-xceiv driver. Signed-off-by: Adam Ford --- drivers/phy/nop-phy.c | 45 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/drivers/phy/nop-phy.c b/drivers/phy/nop-phy.c index 9f12ebc0624..b08eedd4d42 100644 --- a/drivers/phy/nop-phy.c +++ b/drivers/phy/nop-phy.c @@ -10,25 +10,54 @@ #include #include #include +#include struct nop_phy_priv { struct clk_bulk bulk; +#if CONFIG_IS_ENABLED(DM_GPIO) + struct gpio_desc reset_gpio; +#endif }; +#if CONFIG_IS_ENABLED(DM_GPIO) +static int nop_phy_reset(struct phy *phy) +{ + struct nop_phy_priv *priv = dev_get_priv(phy->dev); + + /* Return if there is no gpio since it's optional */ + if (!dm_gpio_is_valid(&priv->reset_gpio)) + return 0; + + return dm_gpio_set_value(&priv->reset_gpio, false); +} +#endif + static int nop_phy_init(struct phy *phy) { struct nop_phy_priv *priv = dev_get_priv(phy->dev); + int ret = 0; - if (CONFIG_IS_ENABLED(CLK)) - return clk_enable_bulk(&priv->bulk); + if (CONFIG_IS_ENABLED(CLK)) { + ret = clk_enable_bulk(&priv->bulk); + if (ret) + return ret; + } +#if CONFIG_IS_ENABLED(DM_GPIO) + ret = nop_phy_reset(phy); + if (ret) { + if (CONFIG_IS_ENABLED(CLK)) + clk_disable_bulk(&priv->bulk); + return ret; + } +#endif return 0; } static int nop_phy_probe(struct udevice *dev) { struct nop_phy_priv *priv = dev_get_priv(dev); - int ret; + int ret = 0; if (CONFIG_IS_ENABLED(CLK)) { ret = clk_get_bulk(dev, &priv->bulk); @@ -37,6 +66,13 @@ static int nop_phy_probe(struct udevice *dev) return ret; } } +#if CONFIG_IS_ENABLED(DM_GPIO) + ret = gpio_request_by_name(dev, "reset-gpios", 0, + &priv->reset_gpio, + GPIOD_IS_OUT); +#endif + if (ret != -ENOENT) + return ret; return 0; } @@ -49,6 +85,9 @@ static const struct udevice_id nop_phy_ids[] = { static struct phy_ops nop_phy_ops = { .init = nop_phy_init, +#if CONFIG_IS_ENABLED(DM_GPIO) + .reset = nop_phy_reset, +#endif }; U_BOOT_DRIVER(nop_phy) = { From 73cde90c8badbeba32524c2708d26fea805fba1e Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 29 Jan 2022 16:33:16 +0100 Subject: [PATCH 18/18] test: test field truncation in snprint() The output size for snprint() should not only be respected for whole fields but also with fields. Add more tests. Signed-off-by: Heinrich Schuchardt --- test/print_ut.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/print_ut.c b/test/print_ut.c index a133907674d..247011f2db0 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -370,6 +370,18 @@ static int snprint(struct unit_test_state *uts) char buf[10] = "xxxxxxxxx"; int ret; + ret = snprintf(buf, 5, "%d", 12345678); + ut_asserteq_str("1234", buf); + ut_asserteq(8, ret); + ret = snprintf(buf, 5, "0x%x", 0x1234); + ut_asserteq_str("0x12", buf); + ut_asserteq(6, ret); + ret = snprintf(buf, 5, "0x%08x", 0x1234); + ut_asserteq_str("0x00", buf); + ut_asserteq(10, ret); + ret = snprintf(buf, 3, "%s", "abc"); + ut_asserteq_str("ab", buf); + ut_asserteq(3, ret); ret = snprintf(buf, 4, "%s:%s", "abc", "def"); ut_asserteq(0, buf[3]); ut_asserteq(7, ret);