zboot: [PATCH] boot/pxe-utils: populate initrd_filesize for extlinux boot
The reason for this is that initrd_filesize is constantly equal to zero or more specifically, potentially uninitialized memory.
I believe this was introduced in 085cbdafca9c3d7bc2f27523a343f61db82f2ccb ("pxe: simplify label_boot()"), diff here:
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index b08aee9896..defbe465e4 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -532,11 +532,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) }
initrd_addr_str = env_get("ramdisk_addr_r"); - strcpy(initrd_filesize, simple_xtoa(size)); - - strncpy(initrd_str, initrd_addr_str, 18); - strcat(initrd_str, ":"); - strncat(initrd_str, initrd_filesize, 9); + size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx", + initrd_addr_str, size); + if (size >= sizeof(initrd_str)) + return 1; }
if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r",
The initrd_filesize completely disappears.
We re-copy the size information inside initrd_filesize, maybe, too naively, something may have to be done to reduce the overflow potential if it exist at all.
pxe_utils.c | 2 ++ 1 file changed, 2 insertions(+)
Currently, it seems like the `initrd_filesize` was uninitialized for a while.
This is particularly problematic when attempting to `zboot` with a initrd with a size coming from `label->initrd`, because it will provide you with a 0-long initrd at boot time, making the kernel fail to continue the boot.
This fixes the issue and I confirmed it enable me booting a U-Boot on QEMU x86_64 q35 with NixOS kernel and initrds.
Signed-off-by: Ryan Lahfa ryan-uboot@lahfa.xyz Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Bin Meng bmeng.cn@gmail.com Cc: Zhaofeng Li hello@zhaofeng.li Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ramon Fried rfried.dev@gmail.com Cc: Artem Lapkin email2tema@gmail.com --- boot/pxe_utils.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index d13c47dd94..fa5e88ab95 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -556,6 +556,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) }
initrd_addr_str = env_get("ramdisk_addr_r"); + /* Copy the actual initrd size inside the initrd_filesize */ + snprintf(initrd_filesize, sizeof(initrd_filesize), "%lx", size); size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx", initrd_addr_str, size); if (size >= sizeof(initrd_str))
On Sat, Sep 16, 2023 at 03:14:58PM +0200, Ryan Lahfa wrote:
Currently, it seems like the `initrd_filesize` was uninitialized for a while.
This is particularly problematic when attempting to `zboot` with a initrd with a size coming from `label->initrd`, because it will provide you with a 0-long initrd at boot time, making the kernel fail to continue the boot.
This fixes the issue and I confirmed it enable me booting a U-Boot on QEMU x86_64 q35 with NixOS kernel and initrds.
The original message was lost because I do not use `git send-email` that often but, this issue appears to be introduced in 085cbdafca9c3d7bc2f27523a343f61db82f2ccb ("pxe: simplify label_boot()").
Kind regards,
On Sat, Sep 16, 2023 at 03:14:58PM +0200, Ryan Lahfa wrote:
Currently, it seems like the `initrd_filesize` was uninitialized for a while.
This is particularly problematic when attempting to `zboot` with a initrd with a size coming from `label->initrd`, because it will provide you with a 0-long initrd at boot time, making the kernel fail to continue the boot.
This fixes the issue and I confirmed it enable me booting a U-Boot on QEMU x86_64 q35 with NixOS kernel and initrds.
Signed-off-by: Ryan Lahfa ryan-uboot@lahfa.xyz Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Bin Meng bmeng.cn@gmail.com Cc: Zhaofeng Li hello@zhaofeng.li Cc: Heinrich Schuchardt heinrich.schuchardt@canonical.com Cc: Ramon Fried rfried.dev@gmail.com Cc: Artem Lapkin email2tema@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
And since you mentioned it in the follow-up, here's the tag so patchwork will pick it up:
Fixes: 085cbdafca9c ("pxe: simplify label_boot()")
Thanks!
+Bin Meng since I think there is a similar patch applied to x86 already?
On Sat, 16 Sept 2023 at 07:31, Ryan Lahfa ryan-uboot@lahfa.xyz wrote:
The reason for this is that initrd_filesize is constantly equal to zero or more specifically, potentially uninitialized memory.
I believe this was introduced in 085cbdafca9c3d7bc2f27523a343f61db82f2ccb ("pxe: simplify label_boot()"), diff here:
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index b08aee9896..defbe465e4 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -532,11 +532,10 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) }
initrd_addr_str = env_get("ramdisk_addr_r");
strcpy(initrd_filesize, simple_xtoa(size));strncpy(initrd_str, initrd_addr_str, 18);strcat(initrd_str, ":");strncat(initrd_str, initrd_filesize, 9);
size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx",initrd_addr_str, size);if (size >= sizeof(initrd_str))return 1; } if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r",The initrd_filesize completely disappears.
We re-copy the size information inside initrd_filesize, maybe, too naively, something may have to be done to reduce the overflow potential if it exist at all.
pxe_utils.c | 2 ++ 1 file changed, 2 insertions(+)
participants (3)
-
Ryan Lahfa -
Simon Glass -
Tom Rini