diff mbox series

[v2,09/11] libnet: Add support for DHCPv4 options 209 and 210

Message ID 1526658340-1992-10-git-send-email-thuth@redhat.com
State Superseded
Headers show
Series Support network booting with pxelinux.cfg files | expand

Commit Message

Thomas Huth May 18, 2018, 3:45 p.m. UTC 
There are two dedicated DHCP options for loading PXELINUX config files,
option 209 (config file name) and 210 (path prefix). We should support
them, too, in case some users want to configure their boot flow this way.
See RFC 5071 and the following URL for more details:
https://www.syslinux.org/wiki/index.php?title=PXELINUX#DHCP_options

Unlike most other strings in libnet, I've chosen to not use fixed-size
arrays for these two strings, but to allocate the memory via malloc here.
We always have to make sure not to overflow the stack in Paflof, so
adding 2 * 256 byte arrays to struct filename_ip sounded just too
dangerous to me.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 lib/libnet/dhcp.c     | 33 +++++++++++++++++++++++++++++++++
 lib/libnet/netload.c  |  7 ++++++-
 lib/libnet/ping.c     | 14 +++++++++-----
 lib/libnet/pxelinux.c | 24 +++++++++++++++++++-----
 lib/libnet/tftp.h     |  2 ++
 5 files changed, 69 insertions(+), 11 deletions(-)

Comments

Alexey Kardashevskiy May 25, 2018, 6:47 a.m. UTC | #1 
On 19/5/18 1:45 am, Thomas Huth wrote:
> There are two dedicated DHCP options for loading PXELINUX config files,
> option 209 (config file name) and 210 (path prefix). We should support
> them, too, in case some users want to configure their boot flow this way.
> See RFC 5071 and the following URL for more details:
> https://www.syslinux.org/wiki/index.php?title=PXELINUX#DHCP_options
> 
> Unlike most other strings in libnet, I've chosen to not use fixed-size
> arrays for these two strings, but to allocate the memory via malloc here.
> We always have to make sure not to overflow the stack in Paflof, so
> adding 2 * 256 byte arrays to struct filename_ip sounded just too
> dangerous to me.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  lib/libnet/dhcp.c     | 33 +++++++++++++++++++++++++++++++++
>  lib/libnet/netload.c  |  7 ++++++-
>  lib/libnet/ping.c     | 14 +++++++++-----
>  lib/libnet/pxelinux.c | 24 +++++++++++++++++++-----
>  lib/libnet/tftp.h     |  2 ++
>  5 files changed, 69 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/libnet/dhcp.c b/lib/libnet/dhcp.c
> index d3e5170..85cd7c0 100644
> --- a/lib/libnet/dhcp.c
> +++ b/lib/libnet/dhcp.c
> @@ -79,6 +79,8 @@
>  #define DHCP_TFTP_SERVER      66
>  #define DHCP_BOOTFILE         67
>  #define DHCP_CLIENT_ARCH      93
> +#define DHCP_PXELINUX_CFGFILE 209   /* See RFC 5071 */
> +#define DHCP_PXELINUX_PREFIX  210
>  #define DHCP_ENDOPT         0xFF
>  #define DHCP_PADOPT         0x00
>  
> @@ -167,6 +169,8 @@ static uint32_t dhcp_siaddr_ip     = 0;
>  static char   dhcp_filename[256];
>  static char   dhcp_tftp_name[256];
>  static uint32_t dhcp_xid;
> +static char *pxelinux_cfgfile;
> +static char *pxelinux_prefix;


Why are not these in dhcp_options_t?



>  static char   * response_buffer;
>  
> @@ -185,6 +189,8 @@ int32_t dhcpv4(char *ret_buffer, filename_ip_t *fn_ip)
>  	strcpy(dhcp_filename, "");
>  	strcpy(dhcp_tftp_name, "");
>  
> +	pxelinux_cfgfile = pxelinux_prefix = NULL;
> +
>  	response_buffer = ret_buffer;
>  
>  	if (dhcp_attempt(fd) == 0)
> @@ -232,6 +238,10 @@ int32_t dhcpv4(char *ret_buffer, filename_ip_t *fn_ip)
>  	fn_ip -> server_ip = dhcp_tftp_ip;
>  	strcpy(fn_ip->filename, dhcp_filename);
>  
> +	fn_ip->pl_cfgfile = pxelinux_cfgfile;
> +	fn_ip->pl_prefix = pxelinux_prefix;
> +	pxelinux_cfgfile = pxelinux_prefix = NULL;
> +
>  	return 0;
>  }
>  
> @@ -456,6 +466,26 @@ static int32_t dhcp_decode_options(uint8_t opt_field[], uint32_t opt_len,
>  			offset += 4;
>  			break;
>  
> +		case DHCP_PXELINUX_CFGFILE:
> +			pxelinux_cfgfile = malloc(opt_field[offset + 1] + 1);
> +			if (pxelinux_cfgfile) {
> +				memcpy(pxelinux_cfgfile, opt_field + offset + 2,
> +				       opt_field[offset + 1]);
> +				pxelinux_cfgfile[opt_field[offset + 1]] = 0;
> +			}
> +			offset += 2 + opt_field[offset + 1];
> +			break;
> +
> +		case DHCP_PXELINUX_PREFIX:
> +			pxelinux_prefix = malloc(opt_field[offset + 1] + 1);
> +			if (pxelinux_prefix) {
> +				memcpy(pxelinux_prefix, opt_field + offset + 2,
> +				       opt_field[offset + 1]);
> +				pxelinux_prefix[opt_field[offset + 1]] = 0;
> +			}
> +			offset += 2 + opt_field[offset + 1];
> +			break;
> +
>  		case DHCP_PADOPT :
>  			offset++;
>  			break;
> @@ -681,6 +711,9 @@ static void dhcp_send_request(int fd)
>  	opt.request_list[DHCP_ROUTER] = 1;
>  	opt.request_list[DHCP_TFTP_SERVER] = 1;
>  	opt.request_list[DHCP_BOOTFILE] = 1;
> +	opt.request_list[DHCP_PXELINUX_CFGFILE] = 1;
> +	opt.request_list[DHCP_PXELINUX_PREFIX] = 1;
> +
>  	opt.request_list[DHCP_CLIENT_ARCH] = USE_DHCPARCH;
>  	opt.flag[DHCP_CLIENT_ARCH] = USE_DHCPARCH;
>  
> diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c
> index 2a2a7c5..5af4e26 100644
> --- a/lib/libnet/netload.c
> +++ b/lib/libnet/netload.c
> @@ -742,7 +742,10 @@ int netload(char *buffer, int len, char *args_fs, int alen)
>  	}
>  
>  	/* Do the TFTP load and print error message if necessary */
> -	rc = tftp_load(&fn_ip, buffer, len);
> +	rc = 0;
> +	if (!fn_ip.pl_cfgfile && strlen(fn_ip.filename) > 0) {
> +		rc = tftp_load(&fn_ip, buffer, len);
> +	}
>  
>  	if (rc <= 0 && !obp_tftp_args.filename[0]) {
>  		rc = net_pxelinux_cfg_load(&fn_ip, buffer, len, own_mac);
> @@ -758,5 +761,7 @@ int netload(char *buffer, int len, char *args_fs, int alen)
>  	}
>    err_out:
>  	SLOF_free_mem(pkt_buffer, MAX_PKT_SIZE);
> +	free(fn_ip.pl_cfgfile);
> +	free(fn_ip.pl_prefix);
>  	return rc;
>  }
> diff --git a/lib/libnet/ping.c b/lib/libnet/ping.c
> index edad5eb..051269f 100644
> --- a/lib/libnet/ping.c
> +++ b/lib/libnet/ping.c
> @@ -115,6 +115,7 @@ int ping(char *args_fs, int alen)
>  	uint8_t own_mac[6];
>  	uint32_t netmask;
>  	char args[256];
> +	int ret = -1;
>  
>  	memset(&ping_args, 0, sizeof(struct ping_args));
>  
> @@ -164,8 +165,7 @@ int ping(char *args_fs, int alen)
>  
>  		if (arp_failed == -1) {
>  			printf("\n  DHCP: Could not get ip address\n");
> -			close(fn_ip.fd);
> -			return -1;
> +			goto free_out;
>  		}
>  
>  	} else {
> @@ -210,12 +210,16 @@ int ping(char *args_fs, int alen)
>  		receive_ether(fd_device);
>  		if(pong_ipv4() == 0) {
>  			printf("success\n");
> -			close(fn_ip.fd);
> -			return 0;
> +			ret = 0;
> +			goto free_out;
>  		}
>  	}
>  
>  	printf("failed\n");
> +free_out:
> +	free(fn_ip.pl_cfgfile);
> +	free(fn_ip.pl_prefix);
>  	close(fn_ip.fd);
> -	return -1;
> +
> +	return ret;
>  }
> diff --git a/lib/libnet/pxelinux.c b/lib/libnet/pxelinux.c
> index 0c0b42e..6c03029 100644
> --- a/lib/libnet/pxelinux.c
> +++ b/lib/libnet/pxelinux.c
> @@ -59,16 +59,30 @@ static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, uint8_t *uuid,
>  	cfgbuf[cfgbufsize - 1] = 0;  /* Make sure it is NUL-terminated */
>  
>  	/* Did we get a usable base directory via DHCP? */
> -	slash = strrchr(fn_ip->filename, '/');
> -	if (slash && slash - fn_ip->filename < sizeof(basedir) - 1) {
> -		slash[1] = 0;
> -		strcpy(basedir, fn_ip->filename);
> +	if (fn_ip->pl_prefix && strlen(fn_ip->pl_prefix) < sizeof(basedir)) {
> +		strcpy(basedir, fn_ip->pl_prefix);
>  	} else {
> -		strcpy(basedir, "pxelinux.cfg/");
> +		slash = strrchr(fn_ip->filename, '/');
> +		if (slash && slash - fn_ip->filename < sizeof(basedir) - 1) {
> +			slash[1] = 0;
> +			strcpy(basedir, fn_ip->filename);
> +		} else {
> +			strcpy(basedir, "pxelinux.cfg/");
> +		}
>  	}
>  
>  	printf("Trying pxelinux.cfg files...\n");
>  
> +	/* Try to load config file according to file name in DHCP option 209 */
> +	if (fn_ip->pl_cfgfile && strlen(fn_ip->pl_cfgfile)
> +	                         + strlen(basedir) < sizeof(fn_ip->filename)) {
> +		sprintf(fn_ip->filename, "%s%s", basedir, fn_ip->pl_cfgfile);
> +		rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize - 1);
> +		if (rc > 0) {
> +			return rc;
> +		}
> +	}
> +
>  	/* Try to load config file with name based on the VM UUID */
>  	if (uuid) {
>  		sprintf(fn_ip->filename, "%s%02x%02x%02x%02x-%02x%02x-"
> diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h
> index da743d3..775700a 100644
> --- a/lib/libnet/tftp.h
> +++ b/lib/libnet/tftp.h
> @@ -29,6 +29,8 @@ struct filename_ip {
>  	ip6_addr_t server_ip6;
>  	ip6_addr_t dns_ip6;
>  	char filename[256];
> +	char *pl_cfgfile; /* For PXELINUX DHCPv4 option 209. Must be free()ed */
> +	char *pl_prefix;  /* For PXELINUX DHCPv4 option 210. Must be free()ed */
>  	int fd;
>  	int ip_version;
>  	int tftp_retries;
>
Thomas Huth May 25, 2018, 7:18 a.m. UTC | #2 
On 25.05.2018 08:47, Alexey Kardashevskiy wrote:
> On 19/5/18 1:45 am, Thomas Huth wrote:
>> There are two dedicated DHCP options for loading PXELINUX config files,
>> option 209 (config file name) and 210 (path prefix). We should support
>> them, too, in case some users want to configure their boot flow this way.
>> See RFC 5071 and the following URL for more details:
>> https://www.syslinux.org/wiki/index.php?title=PXELINUX#DHCP_options
>>
>> Unlike most other strings in libnet, I've chosen to not use fixed-size
>> arrays for these two strings, but to allocate the memory via malloc here.
>> We always have to make sure not to overflow the stack in Paflof, so
>> adding 2 * 256 byte arrays to struct filename_ip sounded just too
>> dangerous to me.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  lib/libnet/dhcp.c     | 33 +++++++++++++++++++++++++++++++++
>>  lib/libnet/netload.c  |  7 ++++++-
>>  lib/libnet/ping.c     | 14 +++++++++-----
>>  lib/libnet/pxelinux.c | 24 +++++++++++++++++++-----
>>  lib/libnet/tftp.h     |  2 ++
>>  5 files changed, 69 insertions(+), 11 deletions(-)
>>
>> diff --git a/lib/libnet/dhcp.c b/lib/libnet/dhcp.c
>> index d3e5170..85cd7c0 100644
>> --- a/lib/libnet/dhcp.c
>> +++ b/lib/libnet/dhcp.c
>> @@ -79,6 +79,8 @@
>>  #define DHCP_TFTP_SERVER      66
>>  #define DHCP_BOOTFILE         67
>>  #define DHCP_CLIENT_ARCH      93
>> +#define DHCP_PXELINUX_CFGFILE 209   /* See RFC 5071 */
>> +#define DHCP_PXELINUX_PREFIX  210
>>  #define DHCP_ENDOPT         0xFF
>>  #define DHCP_PADOPT         0x00
>>  
>> @@ -167,6 +169,8 @@ static uint32_t dhcp_siaddr_ip     = 0;
>>  static char   dhcp_filename[256];
>>  static char   dhcp_tftp_name[256];
>>  static uint32_t dhcp_xid;
>> +static char *pxelinux_cfgfile;
>> +static char *pxelinux_prefix;
> 
> Why are not these in dhcp_options_t?

That whole dhcp code is a little bit strange: dhcp_options_t is only
"valid" during handle_dhcp(), so it finally copies the options also into
the static char dhcp_filename[256] and the static char
dhcp_tftp_name[256] - and dhcpv4() finally copies the stuff over into
filename_ip_t. So the "static" variables are urgently needed for the
currently logic here, while an entry in dhcp_options_t is just optional.
Thus let's keep it simple and only use the static variables here.

 Thomas


PS:
If somebody gets bored, I think a major refactoring of the dhcp code
could make this dhcp.c file way more readable. E.g. I also have not
understood yet why dhcp_encode_options() is checking flag[DHCP_BOOTFILE]
and flag[DHCP_TFTP_SERVER] for example...
diff mbox series

Patch

diff --git a/lib/libnet/dhcp.c b/lib/libnet/dhcp.c
index d3e5170..85cd7c0 100644
--- a/lib/libnet/dhcp.c
+++ b/lib/libnet/dhcp.c
@@ -79,6 +79,8 @@ 
 #define DHCP_TFTP_SERVER      66
 #define DHCP_BOOTFILE         67
 #define DHCP_CLIENT_ARCH      93
+#define DHCP_PXELINUX_CFGFILE 209   /* See RFC 5071 */
+#define DHCP_PXELINUX_PREFIX  210
 #define DHCP_ENDOPT         0xFF
 #define DHCP_PADOPT         0x00
 
@@ -167,6 +169,8 @@  static uint32_t dhcp_siaddr_ip     = 0;
 static char   dhcp_filename[256];
 static char   dhcp_tftp_name[256];
 static uint32_t dhcp_xid;
+static char *pxelinux_cfgfile;
+static char *pxelinux_prefix;
 
 static char   * response_buffer;
 
@@ -185,6 +189,8 @@  int32_t dhcpv4(char *ret_buffer, filename_ip_t *fn_ip)
 	strcpy(dhcp_filename, "");
 	strcpy(dhcp_tftp_name, "");
 
+	pxelinux_cfgfile = pxelinux_prefix = NULL;
+
 	response_buffer = ret_buffer;
 
 	if (dhcp_attempt(fd) == 0)
@@ -232,6 +238,10 @@  int32_t dhcpv4(char *ret_buffer, filename_ip_t *fn_ip)
 	fn_ip -> server_ip = dhcp_tftp_ip;
 	strcpy(fn_ip->filename, dhcp_filename);
 
+	fn_ip->pl_cfgfile = pxelinux_cfgfile;
+	fn_ip->pl_prefix = pxelinux_prefix;
+	pxelinux_cfgfile = pxelinux_prefix = NULL;
+
 	return 0;
 }
 
@@ -456,6 +466,26 @@  static int32_t dhcp_decode_options(uint8_t opt_field[], uint32_t opt_len,
 			offset += 4;
 			break;
 
+		case DHCP_PXELINUX_CFGFILE:
+			pxelinux_cfgfile = malloc(opt_field[offset + 1] + 1);
+			if (pxelinux_cfgfile) {
+				memcpy(pxelinux_cfgfile, opt_field + offset + 2,
+				       opt_field[offset + 1]);
+				pxelinux_cfgfile[opt_field[offset + 1]] = 0;
+			}
+			offset += 2 + opt_field[offset + 1];
+			break;
+
+		case DHCP_PXELINUX_PREFIX:
+			pxelinux_prefix = malloc(opt_field[offset + 1] + 1);
+			if (pxelinux_prefix) {
+				memcpy(pxelinux_prefix, opt_field + offset + 2,
+				       opt_field[offset + 1]);
+				pxelinux_prefix[opt_field[offset + 1]] = 0;
+			}
+			offset += 2 + opt_field[offset + 1];
+			break;
+
 		case DHCP_PADOPT :
 			offset++;
 			break;
@@ -681,6 +711,9 @@  static void dhcp_send_request(int fd)
 	opt.request_list[DHCP_ROUTER] = 1;
 	opt.request_list[DHCP_TFTP_SERVER] = 1;
 	opt.request_list[DHCP_BOOTFILE] = 1;
+	opt.request_list[DHCP_PXELINUX_CFGFILE] = 1;
+	opt.request_list[DHCP_PXELINUX_PREFIX] = 1;
+
 	opt.request_list[DHCP_CLIENT_ARCH] = USE_DHCPARCH;
 	opt.flag[DHCP_CLIENT_ARCH] = USE_DHCPARCH;
 
diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c
index 2a2a7c5..5af4e26 100644
--- a/lib/libnet/netload.c
+++ b/lib/libnet/netload.c
@@ -742,7 +742,10 @@  int netload(char *buffer, int len, char *args_fs, int alen)
 	}
 
 	/* Do the TFTP load and print error message if necessary */
-	rc = tftp_load(&fn_ip, buffer, len);
+	rc = 0;
+	if (!fn_ip.pl_cfgfile && strlen(fn_ip.filename) > 0) {
+		rc = tftp_load(&fn_ip, buffer, len);
+	}
 
 	if (rc <= 0 && !obp_tftp_args.filename[0]) {
 		rc = net_pxelinux_cfg_load(&fn_ip, buffer, len, own_mac);
@@ -758,5 +761,7 @@  int netload(char *buffer, int len, char *args_fs, int alen)
 	}
   err_out:
 	SLOF_free_mem(pkt_buffer, MAX_PKT_SIZE);
+	free(fn_ip.pl_cfgfile);
+	free(fn_ip.pl_prefix);
 	return rc;
 }
diff --git a/lib/libnet/ping.c b/lib/libnet/ping.c
index edad5eb..051269f 100644
--- a/lib/libnet/ping.c
+++ b/lib/libnet/ping.c
@@ -115,6 +115,7 @@  int ping(char *args_fs, int alen)
 	uint8_t own_mac[6];
 	uint32_t netmask;
 	char args[256];
+	int ret = -1;
 
 	memset(&ping_args, 0, sizeof(struct ping_args));
 
@@ -164,8 +165,7 @@  int ping(char *args_fs, int alen)
 
 		if (arp_failed == -1) {
 			printf("\n  DHCP: Could not get ip address\n");
-			close(fn_ip.fd);
-			return -1;
+			goto free_out;
 		}
 
 	} else {
@@ -210,12 +210,16 @@  int ping(char *args_fs, int alen)
 		receive_ether(fd_device);
 		if(pong_ipv4() == 0) {
 			printf("success\n");
-			close(fn_ip.fd);
-			return 0;
+			ret = 0;
+			goto free_out;
 		}
 	}
 
 	printf("failed\n");
+free_out:
+	free(fn_ip.pl_cfgfile);
+	free(fn_ip.pl_prefix);
 	close(fn_ip.fd);
-	return -1;
+
+	return ret;
 }
diff --git a/lib/libnet/pxelinux.c b/lib/libnet/pxelinux.c
index 0c0b42e..6c03029 100644
--- a/lib/libnet/pxelinux.c
+++ b/lib/libnet/pxelinux.c
@@ -59,16 +59,30 @@  static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, uint8_t *uuid,
 	cfgbuf[cfgbufsize - 1] = 0;  /* Make sure it is NUL-terminated */
 
 	/* Did we get a usable base directory via DHCP? */
-	slash = strrchr(fn_ip->filename, '/');
-	if (slash && slash - fn_ip->filename < sizeof(basedir) - 1) {
-		slash[1] = 0;
-		strcpy(basedir, fn_ip->filename);
+	if (fn_ip->pl_prefix && strlen(fn_ip->pl_prefix) < sizeof(basedir)) {
+		strcpy(basedir, fn_ip->pl_prefix);
 	} else {
-		strcpy(basedir, "pxelinux.cfg/");
+		slash = strrchr(fn_ip->filename, '/');
+		if (slash && slash - fn_ip->filename < sizeof(basedir) - 1) {
+			slash[1] = 0;
+			strcpy(basedir, fn_ip->filename);
+		} else {
+			strcpy(basedir, "pxelinux.cfg/");
+		}
 	}
 
 	printf("Trying pxelinux.cfg files...\n");
 
+	/* Try to load config file according to file name in DHCP option 209 */
+	if (fn_ip->pl_cfgfile && strlen(fn_ip->pl_cfgfile)
+	                         + strlen(basedir) < sizeof(fn_ip->filename)) {
+		sprintf(fn_ip->filename, "%s%s", basedir, fn_ip->pl_cfgfile);
+		rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize - 1);
+		if (rc > 0) {
+			return rc;
+		}
+	}
+
 	/* Try to load config file with name based on the VM UUID */
 	if (uuid) {
 		sprintf(fn_ip->filename, "%s%02x%02x%02x%02x-%02x%02x-"
diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h
index da743d3..775700a 100644
--- a/lib/libnet/tftp.h
+++ b/lib/libnet/tftp.h
@@ -29,6 +29,8 @@  struct filename_ip {
 	ip6_addr_t server_ip6;
 	ip6_addr_t dns_ip6;
 	char filename[256];
+	char *pl_cfgfile; /* For PXELINUX DHCPv4 option 209. Must be free()ed */
+	char *pl_prefix;  /* For PXELINUX DHCPv4 option 210. Must be free()ed */
 	int fd;
 	int ip_version;
 	int tftp_retries;