[PATCH] Revert "cmd: pxe: use strdup to copy config" et al
Quentin Schulz
quentin.schulz at theobroma-systems.com
Tue Dec 13 14:45:41 CET 2022
Hi Tom,
On 12/13/22 14:37, Tom Rini wrote:
> This reverts commits
> 51c5c28af59c ("cmd: pxe: use strdup to copy config"),
> a5dacef7380e ("cmd: pxe: support INITRD and FDT selection with FIT") and
> f723c2778cf8 ("cmd: pxe: reorder kernel treatment in label_boot").
>
> As reported by Quentin Schulz, this introduces a regression with
> previously generated and working extlinux.conf files.
>
For reference:
https://lore.kernel.org/u-boot/b7e891d1-d134-b489-eb2d-6125d4c7b6c6@theobroma-systems.com/
for my worry
https://lore.kernel.org/u-boot/f0dd213c-4a34-926d-3f3b-f2ed49bb92c3@linaro.org/
for confirmation from Neil it is breaking existing extlinux.conf
Thanks,
Quentin
> Cc: Neil Armstrong <neil.armstrong at linaro.org>
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> Reported-by: Quentin Schulz <quentin.schulz at theobroma-systems.com>
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
> boot/pxe_utils.c | 69 ++++++++++++++++++++-------------------------
> doc/README.pxe | 8 ------
> include/pxe_utils.h | 2 --
> 3 files changed, 30 insertions(+), 49 deletions(-)
>
> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> index 272431381763..075a0f28309e 100644
> --- a/boot/pxe_utils.c
> +++ b/boot/pxe_utils.c
> @@ -258,7 +258,6 @@ static struct pxe_label *label_create(void)
> static void label_destroy(struct pxe_label *label)
> {
> free(label->name);
> - free(label->kernel_label);
> free(label->kernel);
> free(label->config);
> free(label->append);
> @@ -522,44 +521,28 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
> return 1;
> }
>
> - if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r",
> - NULL) < 0) {
> - printf("Skipping %s for failure retrieving kernel\n",
> - label->name);
> - return 1;
> - }
> -
> - kernel_addr = env_get("kernel_addr_r");
> - /* for FIT, append the configuration identifier */
> - if (label->config) {
> - int len = strlen(kernel_addr) + strlen(label->config) + 1;
> -
> - fit_addr = malloc(len);
> - if (!fit_addr) {
> - printf("malloc fail (FIT address)\n");
> - return 1;
> - }
> - snprintf(fit_addr, len, "%s%s", kernel_addr, label->config);
> - kernel_addr = fit_addr;
> - }
> -
> - /* For FIT, the label can be identical to kernel one */
> - if (label->initrd && !strcmp(label->kernel_label, label->initrd)) {
> - initrd_addr_str = kernel_addr;
> - } else if (label->initrd) {
> + if (label->initrd) {
> ulong size;
> +
> if (get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r",
> &size) < 0) {
> printf("Skipping %s for failure retrieving initrd\n",
> label->name);
> - goto cleanup;
> + return 1;
> }
>
> initrd_addr_str = env_get("ramdisk_addr_r");
> size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx",
> initrd_addr_str, size);
> if (size >= sizeof(initrd_str))
> - goto cleanup;
> + return 1;
> + }
> +
> + if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r",
> + NULL) < 0) {
> + printf("Skipping %s for failure retrieving kernel\n",
> + label->name);
> + return 1;
> }
>
> if (label->ipappend & 0x1) {
> @@ -589,7 +572,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
> strlen(label->append ?: ""),
> strlen(ip_str), strlen(mac_str),
> sizeof(bootargs));
> - goto cleanup;
> + return 1;
> }
>
> if (label->append)
> @@ -604,6 +587,21 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
> printf("append: %s\n", finalbootargs);
> }
>
> + kernel_addr = env_get("kernel_addr_r");
> +
> + /* for FIT, append the configuration identifier */
> + if (label->config) {
> + int len = strlen(kernel_addr) + strlen(label->config) + 1;
> +
> + fit_addr = malloc(len);
> + if (!fit_addr) {
> + printf("malloc fail (FIT address)\n");
> + return 1;
> + }
> + snprintf(fit_addr, len, "%s%s", kernel_addr, label->config);
> + kernel_addr = fit_addr;
> + }
> +
> /*
> * fdt usage is optional:
> * It handles the following scenarios.
> @@ -625,11 +623,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
> */
> bootm_argv[3] = env_get("fdt_addr_r");
>
> - /* For FIT, the label can be identical to kernel one */
> - if (label->fdt && !strcmp(label->kernel_label, label->fdt)) {
> - bootm_argv[3] = kernel_addr;
> /* if fdt label is defined then get fdt from server */
> - } else if (bootm_argv[3]) {
> + if (bootm_argv[3]) {
> char *fdtfile = NULL;
> char *fdtfilefree = NULL;
>
> @@ -1170,19 +1165,15 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
> if (err < 0)
> return err;
>
> - /* copy the kernel label to compare with FDT / INITRD when FIT is used */
> - label->kernel_label = strdup(label->kernel);
> - if (!label->kernel_label)
> - return -ENOMEM;
> -
> s = strstr(label->kernel, "#");
> if (!s)
> return 1;
>
> - label->config = strdup(s);
> + label->config = malloc(strlen(s) + 1);
> if (!label->config)
> return -ENOMEM;
>
> + strcpy(label->config, s);
> *s = 0;
>
> return 1;
> diff --git a/doc/README.pxe b/doc/README.pxe
> index 172201093d02..d14d2bdcc9b0 100644
> --- a/doc/README.pxe
> +++ b/doc/README.pxe
> @@ -179,19 +179,11 @@ initrd <path> - if this label is chosen, use tftp to retrieve the initrd
> at <path>. it will be stored at the address indicated in
> the initrd_addr_r environment variable, and that address
> will be passed to bootm.
> - For FIT image, the initrd can be provided with the same value than
> - kernel, including configuration:
> - <path>#<conf>[#<extra-conf[#...]]
> - In this case, kernel_addr_r is passed to bootm.
>
> fdt <path> - if this label is chosen, use tftp to retrieve the fdt blob
> at <path>. it will be stored at the address indicated in
> the fdt_addr_r environment variable, and that address will
> be passed to bootm.
> - For FIT image, the device tree can be provided with the same value
> - than kernel, including configuration:
> - <path>#<conf>[#<extra-conf[#...]]
> - In this case, kernel_addr_r is passed to bootm.
>
> devicetree <path> - if this label is chosen, use tftp to retrieve the fdt blob
> at <path>. it will be stored at the address indicated in
> diff --git a/include/pxe_utils.h b/include/pxe_utils.h
> index 1e5e8424f53f..4a73b2aace34 100644
> --- a/include/pxe_utils.h
> +++ b/include/pxe_utils.h
> @@ -28,7 +28,6 @@
> * Create these with the 'label_create' function given below.
> *
> * name - the name of the menu as given on the 'menu label' line.
> - * kernel_label - the kernel label, including FIT config if present.
> * kernel - the path to the kernel file to use for this label.
> * append - kernel command line to use when booting this label
> * initrd - path to the initrd to use for this label.
> @@ -41,7 +40,6 @@ struct pxe_label {
> char num[4];
> char *name;
> char *menu;
> - char *kernel_label;
> char *kernel;
> char *config;
> char *append;
More information about the U-Boot
mailing list