Browse Source

[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>
tags/v1.20.1
Michael Brown 9 years ago
parent
commit
0af0888832
1 changed files with 13 additions and 13 deletions
  1. 13
    13
      src/net/udp/tftp.c

+ 13
- 13
src/net/udp/tftp.c View File

1180
  */
1180
  */
1181
 static int tftp_apply_settings ( void ) {
1181
 static int tftp_apply_settings ( void ) {
1182
 	static struct in_addr tftp_server = { 0 };
1182
 	static struct in_addr tftp_server = { 0 };
1183
-	struct in_addr last_tftp_server;
1183
+	struct in_addr new_tftp_server;
1184
 	char uri_string[32];
1184
 	char uri_string[32];
1185
 	struct uri *uri;
1185
 	struct uri *uri;
1186
 
1186
 
1187
 	/* Retrieve TFTP server setting */
1187
 	/* Retrieve TFTP server setting */
1188
-	last_tftp_server = tftp_server;
1189
-	fetch_ipv4_setting ( NULL, &next_server_setting, &tftp_server );
1188
+	fetch_ipv4_setting ( NULL, &next_server_setting, &new_tftp_server );
1190
 
1189
 
1191
 	/* If TFTP server setting has changed, set the current working
1190
 	/* If TFTP server setting has changed, set the current working
1192
 	 * URI to match.  Do it only when the TFTP server has changed
1191
 	 * URI to match.  Do it only when the TFTP server has changed
1195
 	 * an unrelated setting and triggered all the settings
1194
 	 * an unrelated setting and triggered all the settings
1196
 	 * applicators.
1195
 	 * applicators.
1197
 	 */
1196
 	 */
1198
-	if ( tftp_server.s_addr != last_tftp_server.s_addr ) {
1199
-		if ( tftp_server.s_addr ) {
1200
-			snprintf ( uri_string, sizeof ( uri_string ),
1201
-				   "tftp://%s/", inet_ntoa ( tftp_server ) );
1202
-			uri = parse_uri ( uri_string );
1203
-			if ( ! uri )
1204
-				return -ENOMEM;
1205
-		} else {
1206
-			uri = NULL;
1207
-		}
1197
+	if ( new_tftp_server.s_addr &&
1198
+	     ( new_tftp_server.s_addr != tftp_server.s_addr ) ) {
1199
+		DBGC ( &tftp_server, "TFTP server changed %s => ",
1200
+		       inet_ntoa ( tftp_server ) );
1201
+		DBGC ( &tftp_server, "%s\n", inet_ntoa ( new_tftp_server ) );
1202
+		snprintf ( uri_string, sizeof ( uri_string ),
1203
+			   "tftp://%s/", inet_ntoa ( new_tftp_server ) );
1204
+		uri = parse_uri ( uri_string );
1205
+		if ( ! uri )
1206
+			return -ENOMEM;
1208
 		churi ( uri );
1207
 		churi ( uri );
1209
 		uri_put ( uri );
1208
 		uri_put ( uri );
1209
+		tftp_server = new_tftp_server;
1210
 	}
1210
 	}
1211
 
1211
 
1212
 	return 0;
1212
 	return 0;

Loading…
Cancel
Save