[tftp] Do not change current working URI when TFTP server is cleared

For historical reasons, iPXE sets the current working URI to the root
of the TFTP server whenever the TFTP server address is changed.  This
was originally implemented in the hope of allowing a DHCP-provided
TFTP filename to be treated simply as a relative URI.  This usage
turns out to be impractical since DHCP-provided TFTP filenames may
include characters which would have special significance to the URI
parser, and so the DHCP next-server+filename combination is now
handled by the dedicated pxe_uri() function instead.

The practice of setting the current working URI to the root of the
TFTP server is potentially helpful for interactive uses of iPXE,
allowing a user to type e.g.

  iPXE> dhcp
  Configuring (net0 52:54:00:12:34:56)... ok
  iPXE> chain pxelinux.0

and have the URI "pxelinux.0" interpreted as being relative to the
root of the TFTP server provided via DHCP.

The current implementation of tftp_apply_settings() has an unintended
flaw.  When the "dhcp" command is used to renew a DHCP lease (or to
pick up potentially modified DHCP options), the old settings block
will be unregistered before the new settings block is registered.
This causes tftp_apply_settings() to believe that the TFTP server has
been changed twice (to 0.0.0.0 and back again), and so the current
working URI will always be set to the root of the TFTP server, even if
the DHCP response provides exactly the same TFTP server as previously.

Fix by doing nothing in tftp_apply_settings() whenever there is no
TFTP server address.

Debugged-by: Andrew Widdersheim <awiddersheim@inetu.net>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 13 insertions, 13 deletions

diff --git a/src/net/udp/tftp.c b/src/net/udp/tftp.c
index 953bcb46..f3cb3434 100644
--- a/src/net/udp/tftp.c
+++ b/src/net/udp/tftp.c
@@ -1180,13 +1180,12 @@ struct uri_opener mtftp_uri_opener __uri_opener = {
  */
 static int tftp_apply_settings ( void ) {
 	static struct in_addr tftp_server = { 0 };
-	struct in_addr last_tftp_server;
+	struct in_addr new_tftp_server;
 	char uri_string[32];
 	struct uri *uri;
 
 	/* Retrieve TFTP server setting */
-	last_tftp_server = tftp_server;
-	fetch_ipv4_setting ( NULL, &next_server_setting, &tftp_server );
+	fetch_ipv4_setting ( NULL, &next_server_setting, &new_tftp_server );
 
 	/* If TFTP server setting has changed, set the current working
 	 * URI to match.  Do it only when the TFTP server has changed
@@ -1195,18 +1194,19 @@ static int tftp_apply_settings ( void ) {
 	 * an unrelated setting and triggered all the settings
 	 * applicators.
 	 */
-	if ( tftp_server.s_addr != last_tftp_server.s_addr ) {
-		if ( tftp_server.s_addr ) {
-			snprintf ( uri_string, sizeof ( uri_string ),
-				   "tftp://%s/", inet_ntoa ( tftp_server ) );
-			uri = parse_uri ( uri_string );
-			if ( ! uri )
-				return -ENOMEM;
-		} else {
-			uri = NULL;
-		}
+	if ( new_tftp_server.s_addr &&
+	     ( new_tftp_server.s_addr != tftp_server.s_addr ) ) {
+		DBGC ( &tftp_server, "TFTP server changed %s => ",
+		       inet_ntoa ( tftp_server ) );
+		DBGC ( &tftp_server, "%s\n", inet_ntoa ( new_tftp_server ) );
+		snprintf ( uri_string, sizeof ( uri_string ),
+			   "tftp://%s/", inet_ntoa ( new_tftp_server ) );
+		uri = parse_uri ( uri_string );
+		if ( ! uri )
+			return -ENOMEM;
 		churi ( uri );
 		uri_put ( uri );
+		tftp_server = new_tftp_server;
 	}
 
 	return 0;