[PATCH] net: bootp: extend ProxyDHCP support to read bootfile name

[Jerome Forissier]

Hi Ryan,

[Replying from my personal address because for some reason my email is
rejected by the Arm email servers: "messages to GMX.DE; are blocked by
your organization [...]"]

On 5/20/26 23:59, Ryan Persée wrote:
> When CONFIG_SERVERIP_FROM_PROXYDHCP is enabled, a ProxyDHCP server
> responds with a zero yiaddr and supplies PXE-specific parameters such
> as the TFTP server address. However, the bootfile name was only read
> from the BOOTP legacy 'bp_file' header field. Modern ProxyDHCP servers
> (e.g. dnsmasq, iPXE) advertise the bootfile via DHCP option 67 instead,
> which was previously ignored.
> 
> Furthermore, once the ProxyDHCP bootfile was stored, it could be silently
> overwritten by a subsequent DHCP OFFER or ACK from the main DHCP server
> if that packet also carried a bootfile (via option 67 or bp_file).
> 
> This patch makes three minimal changes within the existing
> CONFIG_SERVERIP_FROM_PROXYDHCP guards:
> 
> 1. Add store_proxydhcp_bootfile(), a targeted helper that walks the DHCP
>    vendor options of a ProxyDHCP response and extracts only option 67.
>    All other options are deliberately ignored so that the ProxyDHCP
>    server cannot alter the client's network configuration (gateway, DNS,
>    etc.), which remains the main DHCP server's responsibility.
>    Option 67 takes precedence over the legacy bp_file header field;
>    bp_file is used as a fallback via the existing store_bootp_params().
> 
> 2. Add a !net_boot_file_name[0] guard in store_bootp_params() and in
>    dhcp_process_options() case 67 so that a bootfile already set by the
>    ProxyDHCP response is not overwritten by the main DHCP exchange.
> 
> 3. Extend the ProxyDHCP wait condition in the SELECTING state to also
>    delay when the bootfile has not yet been received, mirroring the
>    existing delay for the server IP address.
> 
> Signed-off-by: Ryan Persée <98691129+rpersee at users.noreply.github.com>
> ---
>  net/bootp.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bootp.c b/net/bootp.c
> index 8976936b184..a16245cc7e8 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -161,7 +161,11 @@ static void store_bootp_params(struct bootp_hdr *bp)
>  	    !(dhcp_option_overload & OVERLOAD_FILE) &&
>  #endif
>  	    (strlen(bp->bp_file) > 0) &&
> -	    !net_boot_file_name_explicit) {
> +	    !net_boot_file_name_explicit
> +#if defined(CONFIG_SERVERIP_FROM_PROXYDHCP)
> +	    && !net_boot_file_name[0]
> +#endif
> +	    ) {
>  		copy_filename(net_boot_file_name, bp->bp_file,
>  			      sizeof(net_boot_file_name));
>  	}
> @@ -935,7 +939,11 @@ static void dhcp_process_options(uchar *popt, uchar *end)
>  		case 66:	/* Ignore TFTP server name */
>  			break;
>  		case 67:	/* Bootfile option */
> -			if (!net_boot_file_name_explicit) {
> +			if (!net_boot_file_name_explicit
> +#if defined(CONFIG_SERVERIP_FROM_PROXYDHCP)
> +			    && !net_boot_file_name[0]
> +#endif
> +			    ) {
>  				size = truncate_sz("Bootfile",
>  						   sizeof(net_boot_file_name),
>  						   oplen);
> @@ -1080,6 +1088,43 @@ static void dhcp_send_request_packet(struct bootp_hdr *bp_offer)
>  	net_send_packet(net_tx_packet, pktlen);
>  }
>  
> +#if defined(CONFIG_SERVERIP_FROM_PROXYDHCP)
> +/*
> + * Extract bootfile name from DHCP option 67 in a ProxyDHCP response.
> + * Only option 67 is read; all other options are intentionally ignored to
> + * avoid letting the ProxyDHCP server alter the client's network config
> + * (gateway, DNS, etc.) which is the main DHCP server's responsibility.
> + */
> +static void store_proxydhcp_bootfile(struct bootp_hdr *bp)
> +{
> +	uchar *popt = (uchar *)&bp->bp_vend[4];
> +	uchar *end = (uchar *)bp + BOOTP_HDR_SIZE;
> +	int oplen, size;
> +
> +	if (net_read_u32((u32 *)&bp->bp_vend[0]) != htonl(BOOTP_VENDOR_MAGIC))
> +		return;
> +
> +	while (popt < end && *popt != 0xff) {
> +		if (*popt == 0) {
> +			popt++;
> +			continue;
> +		}

I believe you need this to avoid a potential OOB read:

		if (popt + 1 >= end)
			break;

> +		oplen = *(popt + 1);
> +		if (popt + 2 + oplen > end)
> +			break;
> +		if (*popt == 67 && !net_boot_file_name_explicit &&
> +		    !net_boot_file_name[0]) {
> +			size = truncate_sz("Bootfile",
> +					   sizeof(net_boot_file_name), oplen);
> +			memcpy(net_boot_file_name, popt + 2, size);
> +			net_boot_file_name[size] = 0;
> +			break;
> +		}
> +		popt += oplen + 2;
> +	}
> +}
> +#endif
> +
>  /*
>   *	Handle DHCP received packets.
>   */
> @@ -1100,6 +1145,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  
>  	if (net_read_ip(&bp->bp_yiaddr).s_addr == 0) {
>  #if defined(CONFIG_SERVERIP_FROM_PROXYDHCP)
> +		store_proxydhcp_bootfile(bp);
>  		store_bootp_params(bp);
>  #endif
>  		return;
> @@ -1130,7 +1176,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  				efi_net_set_dhcp_ack(pkt, len);
>  
>  #if defined(CONFIG_SERVERIP_FROM_PROXYDHCP)
> -			if (!net_server_ip.s_addr)
> +			if (!net_server_ip.s_addr || !net_boot_file_name[0])

Shouldn't !net_boot_file_name[0] be (!net_boot_file_name_explicit &&
!net_boot_file_name[0])?

>  				udelay(CONFIG_SERVERIP_FROM_PROXYDHCP_DELAY_MS *
>  					1000);
>  #endif	/* CONFIG_SERVERIP_FROM_PROXYDHCP */

Other than that, LGTM.

Thanks,
-- 
Jerome