[U-Boot] [PATCH] pxe: clear Bootfile before returning
From: Stephen Warren swarren@nvidia.com
When "pxe boot" downloads the initrd/kernel/DTB, netboot_common() saves the downloaded filename to global variable BootFile. If the boot operation is aborted, this global state is not cleared. If "dhcp" is executed later without any arguments, BootFile is not cleared, and when the DHCP response is received, BootpCopyNetParams() writes the value into environment variable bootfile.
This causes the following scenario:
* Boot script executes dhcp; pxe get; pxe boot
* User CTRL-C's the PXE menu, which causes the first menu item to be booted, which causes some file to be downloaded.
(This boot-on-CTRL-C behaviour is arguably a bug too, but it's a separate bug and the bug this patch fixes would still exist if the user simply waited to press CTRL-C until "pxe boot" started downloading files)
* User CTRL-C's the file downloads, but the filename is still written to the bootfile environment variable.
* User re-runs the boot command, which in my case executes "dhcp; pxe get; pxe boot" again, and "dhcp" picks up the saved bootfile environment variable and proceeds to download a file that it shouldn't.
To solve this, modify the implementation of "pxe get" to clear BootFile if the whole boot operation fails, which avoids this whole mess.
An alternative would be to modify netboot_common() such that the no- arguments case explicitly clears the global variable BootFile. However, that would prevent the following command sequences from working:
$ dhcp filename # downloads "filename" $ dhcp # downloads $bootfile, i.e. "filename"
or: $ setenv bootfile filename $ dhcp # downloads $bootfile, i.e. "filename"
... and I assume someone relies on U-Boot working that way.
Signed-off-by: Stephen Warren swarren@nvidia.com --- common/cmd_pxe.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c index ba48692e8641..28999f573496 100644 --- a/common/cmd_pxe.c +++ b/common/cmd_pxe.c @@ -1554,6 +1554,8 @@ do_pxe_boot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
destroy_pxe_menu(cfg);
+ copy_filename(BootFile, "", sizeof(BootFile)); + return 0; }
On Tue, Jul 22, 2014 at 7:06 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
When "pxe boot" downloads the initrd/kernel/DTB, netboot_common() saves the downloaded filename to global variable BootFile. If the boot operation is aborted, this global state is not cleared. If "dhcp" is executed later without any arguments, BootFile is not cleared, and when the DHCP response is received, BootpCopyNetParams() writes the value into environment variable bootfile.
This causes the following scenario:
Boot script executes dhcp; pxe get; pxe boot
User CTRL-C's the PXE menu, which causes the first menu item to be booted, which causes some file to be downloaded.
(This boot-on-CTRL-C behaviour is arguably a bug too, but it's a separate bug and the bug this patch fixes would still exist if the user simply waited to press CTRL-C until "pxe boot" started downloading files)
User CTRL-C's the file downloads, but the filename is still written to the bootfile environment variable.
User re-runs the boot command, which in my case executes "dhcp; pxe get; pxe boot" again, and "dhcp" picks up the saved bootfile environment variable and proceeds to download a file that it shouldn't.
To solve this, modify the implementation of "pxe get" to clear BootFile if the whole boot operation fails, which avoids this whole mess.
An alternative would be to modify netboot_common() such that the no- arguments case explicitly clears the global variable BootFile. However, that would prevent the following command sequences from working:
$ dhcp filename # downloads "filename" $ dhcp # downloads $bootfile, i.e. "filename"
or: $ setenv bootfile filename $ dhcp # downloads $bootfile, i.e. "filename"
... and I assume someone relies on U-Boot working that way.
Signed-off-by: Stephen Warren swarren@nvidia.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
On 07/23/2014 03:37 PM, Joe Hershberger wrote:
On Tue, Jul 22, 2014 at 7:06 PM, Stephen Warren <swarren@wwwdotorg.org mailto:swarren@wwwdotorg.org> wrote:
From: Stephen Warren <swarren@nvidia.com mailto:swarren@nvidia.com>
When "pxe boot" downloads the initrd/kernel/DTB, netboot_common() saves the downloaded filename to global variable BootFile. If the boot operation is aborted, this global state is not cleared. If "dhcp" is executed later without any arguments, BootFile is not cleared, and when the DHCP response is received, BootpCopyNetParams() writes the value into environment variable bootfile.
...
Acked-by: Joe Hershberger <joe.hershberger@ni.com mailto:joe.hershberger@ni.com>
Thanks. I'm not sure if you ack'd this simply so you'd remember you'd reviewed it, or because you expect someone else to apply the change? If the latter, I should forward the patch to them so they know about it:-)
On Wed, Jul 23, 2014 at 4:41 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 07/23/2014 03:37 PM, Joe Hershberger wrote:
On Tue, Jul 22, 2014 at 7:06 PM, Stephen Warren <swarren@wwwdotorg.org mailto:swarren@wwwdotorg.org> wrote:
From: Stephen Warren <swarren@nvidia.com mailto:swarren@nvidia.com>
When "pxe boot" downloads the initrd/kernel/DTB, netboot_common() saves the downloaded filename to global variable BootFile. If the boot operation is aborted, this global state is not cleared. If "dhcp" is executed later without any arguments, BootFile is not cleared, and when the DHCP response is received, BootpCopyNetParams() writes the value
into
environment variable bootfile.
...
Acked-by: Joe Hershberger <joe.hershberger@ni.com mailto:joe.hershberger@ni.com>
Thanks. I'm not sure if you ack'd this simply so you'd remember you'd reviewed it, or because you expect someone else to apply the change? If the latter, I should forward the patch to them so they know about it:-)
Partly for me to remember and partly because recently Tom has been picking these few net things up directly.
Cheers -Joe
On 07/23/2014 03:56 PM, Joe Hershberger wrote:
On Wed, Jul 23, 2014 at 4:41 PM, Stephen Warren <swarren@wwwdotorg.org mailto:swarren@wwwdotorg.org> wrote:
On 07/23/2014 03:37 PM, Joe Hershberger wrote:
On Tue, Jul 22, 2014 at 7:06 PM, Stephen Warren
<swarren@wwwdotorg.org mailto:swarren@wwwdotorg.org
<mailto:swarren@wwwdotorg.org mailto:swarren@wwwdotorg.org>> wrote:
From: Stephen Warren <swarren@nvidia.com
mailto:swarren@nvidia.com <mailto:swarren@nvidia.com mailto:swarren@nvidia.com>>
When "pxe boot" downloads the initrd/kernel/DTB, netboot_common()
saves
the downloaded filename to global variable BootFile. If the boot operation is aborted, this global state is not cleared. If "dhcp" is executed later without any arguments, BootFile is not cleared, and
when
the DHCP response is received, BootpCopyNetParams() writes the
value into
environment variable bootfile.
...
Acked-by: Joe Hershberger <joe.hershberger@ni.com
<mailto:joe.hershberger@ni.com mailto:joe.hershberger@ni.com>>
Thanks. I'm not sure if you ack'd this simply so you'd remember you'd reviewed it, or because you expect someone else to apply the change? If the latter, I should forward the patch to them so they know about it:-)
Partly for me to remember and partly because recently Tom has been picking these few net things up directly.
Tom, are you planning on picking up this patch?
On Tue, Jul 22, 2014 at 06:06:46PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
When "pxe boot" downloads the initrd/kernel/DTB, netboot_common() saves the downloaded filename to global variable BootFile. If the boot operation is aborted, this global state is not cleared. If "dhcp" is executed later without any arguments, BootFile is not cleared, and when the DHCP response is received, BootpCopyNetParams() writes the value into environment variable bootfile.
This causes the following scenario:
Boot script executes dhcp; pxe get; pxe boot
User CTRL-C's the PXE menu, which causes the first menu item to be booted, which causes some file to be downloaded.
(This boot-on-CTRL-C behaviour is arguably a bug too, but it's a separate bug and the bug this patch fixes would still exist if the user simply waited to press CTRL-C until "pxe boot" started downloading files)
User CTRL-C's the file downloads, but the filename is still written to the bootfile environment variable.
User re-runs the boot command, which in my case executes "dhcp; pxe get; pxe boot" again, and "dhcp" picks up the saved bootfile environment variable and proceeds to download a file that it shouldn't.
To solve this, modify the implementation of "pxe get" to clear BootFile if the whole boot operation fails, which avoids this whole mess.
An alternative would be to modify netboot_common() such that the no- arguments case explicitly clears the global variable BootFile. However, that would prevent the following command sequences from working:
$ dhcp filename # downloads "filename" $ dhcp # downloads $bootfile, i.e. "filename"
or: $ setenv bootfile filename $ dhcp # downloads $bootfile, i.e. "filename"
... and I assume someone relies on U-Boot working that way.
Signed-off-by: Stephen Warren swarren@nvidia.com Acked-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot/master, thanks!
participants (3)
-
Joe Hershberger -
Stephen Warren -
Tom Rini