* [U-Boot] [PATCH] pxe: clear Bootfile before returning
@ 2014-07-23 0:06 Stephen Warren
2014-07-23 21:37 ` Joe Hershberger
2014-08-10 22:21 ` [U-Boot] " Tom Rini
0 siblings, 2 replies; 6+ messages in thread
From: Stephen Warren @ 2014-07-23 0:06 UTC (permalink / raw)
To: u-boot
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;
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] pxe: clear Bootfile before returning
2014-07-23 0:06 [U-Boot] [PATCH] pxe: clear Bootfile before returning Stephen Warren
@ 2014-07-23 21:37 ` Joe Hershberger
2014-07-23 21:41 ` Stephen Warren
2014-08-10 22:21 ` [U-Boot] " Tom Rini
1 sibling, 1 reply; 6+ messages in thread
From: Joe Hershberger @ 2014-07-23 21:37 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] pxe: clear Bootfile before returning
2014-07-23 21:37 ` Joe Hershberger
@ 2014-07-23 21:41 ` Stephen Warren
2014-07-23 21:56 ` Joe Hershberger
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2014-07-23 21:41 UTC (permalink / raw)
To: u-boot
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 at 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:-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] pxe: clear Bootfile before returning
2014-07-23 21:41 ` Stephen Warren
@ 2014-07-23 21:56 ` Joe Hershberger
2014-08-06 15:56 ` Stephen Warren
0 siblings, 1 reply; 6+ messages in thread
From: Joe Hershberger @ 2014-07-23 21:56 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] pxe: clear Bootfile before returning
2014-07-23 21:56 ` Joe Hershberger
@ 2014-08-06 15:56 ` Stephen Warren
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-08-06 15:56 UTC (permalink / raw)
To: u-boot
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 at wwwdotorg.org <mailto:swarren@wwwdotorg.org>
> > > <mailto:swarren at 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 at 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] pxe: clear Bootfile before returning
2014-07-23 0:06 [U-Boot] [PATCH] pxe: clear Bootfile before returning Stephen Warren
2014-07-23 21:37 ` Joe Hershberger
@ 2014-08-10 22:21 ` Tom Rini
1 sibling, 0 replies; 6+ messages in thread
From: Tom Rini @ 2014-08-10 22:21 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140810/432505eb/attachment.pgp>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-08-10 22:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-23 0:06 [U-Boot] [PATCH] pxe: clear Bootfile before returning Stephen Warren
2014-07-23 21:37 ` Joe Hershberger
2014-07-23 21:41 ` Stephen Warren
2014-07-23 21:56 ` Joe Hershberger
2014-08-06 15:56 ` Stephen Warren
2014-08-10 22:21 ` [U-Boot] " Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox