[PATCH v1] net: bootp: bound DHCP option parsing by the received packet length

[PATCH v1] net: bootp: bound DHCP option parsing by the received packet length

[Pranav Rajendran]

dhcp_packet_process_options() derives the end of the option area from
BOOTP_HDR_SIZE, a compile-time constant, rather than from the length of
the packet that was actually received:

	uchar *popt = (uchar *)&bp->bp_vend[4];
	uchar *end = popt + BOOTP_HDR_SIZE;

Since popt already starts near the end of the header, 'end' lands
sizeof(struct bootp_hdr) bytes beyond it, so a short reply leaves
dhcp_process_options() walking off the end of the received data and
into whatever the receive buffer held before - typically the remains of
earlier packets.

That is not only a disclosure: the options found there are acted on
like any others, so stale bytes that happen to parse as an option can
influence the boot file name, the DNS server or the root path.

The BOOTP path already gets this right and passes the real length to
bootp_process_vendor(), and both callers here have the length in scope
- they hand it to dhcp_message_type() on the lines above. Pass it in
and use it as the limit. The overloaded 'file' and 'sname' areas are
clamped the same way, as a truncated packet need not contain them
either.

Fixes: 774c3e05ec0a ("net: parse DHCP options from overloaded file/sname fields")
Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
---
 net/bootp.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/net/bootp.c b/net/bootp.c
index f0dc329d6e4..fdedecb3f50 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -968,32 +968,43 @@ static void dhcp_process_options(uchar *popt, uchar *end)
 	}
 }
 
-static void dhcp_packet_process_options(struct bootp_hdr *bp)
+static void dhcp_packet_process_options(struct bootp_hdr *bp, unsigned int len)
 {
-	uchar *popt = (uchar *)&bp->bp_vend[4];
-	uchar *end = popt + BOOTP_HDR_SIZE;
+	uchar *pkt_end = (uchar *)bp + len;
+	uchar *popt, *end;
+
+	if (len < offsetof(struct bootp_hdr, bp_vend) + 4)
+		return;
 
 	if (net_read_u32((u32 *)&bp->bp_vend[0]) != htonl(BOOTP_VENDOR_MAGIC))
 		return;
 
+	popt = (uchar *)&bp->bp_vend[4];
+
 	dhcp_option_overload = 0;
 
 	/*
 	 * The 'options' field MUST be interpreted first, 'file' next,
 	 * 'sname' last.
 	 */
-	dhcp_process_options(popt, end);
+	dhcp_process_options(popt, pkt_end);
 
 	if (dhcp_option_overload & OVERLOAD_FILE) {
 		popt = (uchar *)bp->bp_file;
 		end = popt + sizeof(bp->bp_file);
-		dhcp_process_options(popt, end);
+		if (end > pkt_end)
+			end = pkt_end;
+		if (popt < end)
+			dhcp_process_options(popt, end);
 	}
 
 	if (dhcp_option_overload & OVERLOAD_SNAME) {
 		popt = (uchar *)bp->bp_sname;
 		end = popt + sizeof(bp->bp_sname);
-		dhcp_process_options(popt, end);
+		if (end > pkt_end)
+			end = pkt_end;
+		if (popt < end)
+			dhcp_process_options(popt, end);
 	}
 }
 
@@ -1124,7 +1135,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 				debug("got BOOTP response; transitioning to BOUND\n");
 				goto dhcp_got_bootp;
 			}
-			dhcp_packet_process_options(bp);
+			dhcp_packet_process_options(bp, len);
 			if (CONFIG_IS_ENABLED(EFI_LOADER) &&
 			    IS_ENABLED(CONFIG_NETDEVICES))
 				efi_net_set_dhcp_ack(pkt, len);
@@ -1151,7 +1162,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 
 		if (dhcp_message_type((u8 *)bp->bp_vend, (u8 *)pkt + len) == DHCP_ACK) {
 dhcp_got_bootp:
-			dhcp_packet_process_options(bp);
+			dhcp_packet_process_options(bp, len);
 			/* Store net params from reply */
 			store_net_params(bp);
 			dhcp_state = BOUND;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v1] net: bootp: bound DHCP option parsing by the received packet length

[Jerome Forissier]

Hi Pranav,

On 16/08/2026 00:08, Pranav Rajendran wrote:
> dhcp_packet_process_options() derives the end of the option area from
> BOOTP_HDR_SIZE, a compile-time constant, rather than from the length of
> the packet that was actually received:
> 
> 	uchar *popt = (uchar *)&bp->bp_vend[4];
> 	uchar *end = popt + BOOTP_HDR_SIZE;
> 
> Since popt already starts near the end of the header, 'end' lands
> sizeof(struct bootp_hdr) bytes beyond it, so a short reply leaves
> dhcp_process_options() walking off the end of the received data and
> into whatever the receive buffer held before - typically the remains of
> earlier packets.
> 
> That is not only a disclosure: the options found there are acted on
> like any others, so stale bytes that happen to parse as an option can
> influence the boot file name, the DNS server or the root path.
> 
> The BOOTP path already gets this right and passes the real length to
> bootp_process_vendor(), and both callers here have the length in scope
> - they hand it to dhcp_message_type() on the lines above. Pass it in
> and use it as the limit. The overloaded 'file' and 'sname' areas are
> clamped the same way, as a truncated packet need not contain them
> either.
> 
> Fixes: 774c3e05ec0a ("net: parse DHCP options from overloaded file/sname fields")
> Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
> ---
>  net/bootp.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/net/bootp.c b/net/bootp.c
> index f0dc329d6e4..fdedecb3f50 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -968,32 +968,43 @@ static void dhcp_process_options(uchar *popt, uchar *end)
>  	}
>  }
>  
> -static void dhcp_packet_process_options(struct bootp_hdr *bp)
> +static void dhcp_packet_process_options(struct bootp_hdr *bp, unsigned int len)
>  {
> -	uchar *popt = (uchar *)&bp->bp_vend[4];
> -	uchar *end = popt + BOOTP_HDR_SIZE;
> +	uchar *pkt_end = (uchar *)bp + len;
> +	uchar *popt, *end;
> +
> +	if (len < offsetof(struct bootp_hdr, bp_vend) + 4)
> +		return;
>  
>  	if (net_read_u32((u32 *)&bp->bp_vend[0]) != htonl(BOOTP_VENDOR_MAGIC))
>  		return;
>  
> +	popt = (uchar *)&bp->bp_vend[4];
> +
>  	dhcp_option_overload = 0;
>  
>  	/*
>  	 * The 'options' field MUST be interpreted first, 'file' next,
>  	 * 'sname' last.
>  	 */
> -	dhcp_process_options(popt, end);
> +	dhcp_process_options(popt, pkt_end);

This fixes the outer bound, but I don't think it is sufficient to make the parser
safe for truncated packets.

dhcp_process_options() currently starts with:

	while (popt < end && *popt != 0xff) {
		oplen = *(popt + 1);

If the received packet ends with a single option code byte, popt < end is true but
popt + 1 is already out of bounds.

There is also no validation that the complete option payload is present before the
switch processes it.

So I think a prerequisite patch is needed to fix dhcp_process_options() first.

Thanks,
-- 
Jerome

>  
>  	if (dhcp_option_overload & OVERLOAD_FILE) {
>  		popt = (uchar *)bp->bp_file;
>  		end = popt + sizeof(bp->bp_file);
> -		dhcp_process_options(popt, end);
> +		if (end > pkt_end)
> +			end = pkt_end;
> +		if (popt < end)
> +			dhcp_process_options(popt, end);
>  	}
>  
>  	if (dhcp_option_overload & OVERLOAD_SNAME) {
>  		popt = (uchar *)bp->bp_sname;
>  		end = popt + sizeof(bp->bp_sname);
> -		dhcp_process_options(popt, end);
> +		if (end > pkt_end)
> +			end = pkt_end;
> +		if (popt < end)
> +			dhcp_process_options(popt, end);
>  	}
>  }
>  
> @@ -1124,7 +1135,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  				debug("got BOOTP response; transitioning to BOUND\n");
>  				goto dhcp_got_bootp;
>  			}
> -			dhcp_packet_process_options(bp);
> +			dhcp_packet_process_options(bp, len);
>  			if (CONFIG_IS_ENABLED(EFI_LOADER) &&
>  			    IS_ENABLED(CONFIG_NETDEVICES))
>  				efi_net_set_dhcp_ack(pkt, len);
> @@ -1151,7 +1162,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  
>  		if (dhcp_message_type((u8 *)bp->bp_vend, (u8 *)pkt + len) == DHCP_ACK) {
>  dhcp_got_bootp:
> -			dhcp_packet_process_options(bp);
> +			dhcp_packet_process_options(bp, len);
>  			/* Store net params from reply */
>  			store_net_params(bp);
>  			dhcp_state = BOUND;

[PATCH v2 0/2] net: bootp: bound DHCP option parsing by the received packet length

[Pranav Rajendran]

Jerome pointed out on v1 that bounding dhcp_packet_process_options()
by the received length isn't enough on its own: dhcp_process_options()
itself reads the option-length byte and the option payload without
checking either is inside [popt, end), so a packet truncated right
after an option code byte (or with a length byte that overruns the
buffer) still walks off the end even with the outer bound fixed.

v2 adds a prerequisite patch (1/2) that closes that inner gap first,
then rebases the original outer-bound fix (2/2, unchanged from v1) on
top of it. With both applied, a short reply now stops parsing at the
first truncated option instead of reading past the receive buffer,
regardless of whether the truncation lands on the outer BOOTP_HDR_SIZE
boundary or inside an individual option.

Both patches are checkpatch --strict clean and build warning-free at
W=1 for net/bootp.o; a full sandbox build also succeeds.

Pranav Rajendran (2):
  net: bootp: validate DHCP option length before parsing it
  net: bootp: bound DHCP option parsing by the received packet length

 net/bootp.c | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH v2 1/2] net: bootp: validate DHCP option length before parsing it

[Pranav Rajendran]

dhcp_process_options() reads the option length byte and dereferences
the option payload without checking either is inside [popt, end):

	while (popt < end && *popt != 0xff) {
		oplen = *(popt + 1);
		switch (*popt) {
		case 0:
			oplen = -1; /* Pad omits len byte */
			break;
		case 1:
			net_copy_ip(&net_netmask, (popt + 2));
		...

The loop guard only proves *popt is readable. If a packet ends right
after an option code byte, popt + 1 is already one past the received
data, so oplen = *(popt + 1) reads out of bounds. The pad case (0)
hits this unconditionally, since oplen is read before the switch
even determines the option is a pad.

Once oplen is read, nothing checks that popt + 2 + oplen - the option
header plus its declared payload - is still within end before the
switch dereferences popt + 2 onward (net_copy_ip, memcpy, strlcpy,
the option-52 overload byte, and the PXE config file allocation all
do this). A short final option with an oplen that overruns the buffer
is processed as if the payload were present, so out-of-bounds bytes
are copied into net_netmask, net_root_path, dhcp_option_overload, and
similar globals that go on to influence boot behaviour.

Handle the pad option before touching a second byte, require a length
byte to exist before reading it, and require the full declared option
(header + payload) to fit before entering the switch. A truncated
trailing option now stops parsing instead of reading past the buffer.

This is a prerequisite for bounding dhcp_process_options() by the
received packet length rather than by BOOTP_HDR_SIZE: fixing the outer
limit alone leaves this inner out-of-bounds read reachable whenever a
short reply's last option is cut off before its length or payload
bytes.

Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
---
v2: New patch, added in response to review feedback on v1 of
    "net: bootp: bound DHCP option parsing by the received packet
    length" pointing out this inner gap.

 net/bootp.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/bootp.c b/net/bootp.c
index f0dc329d6e4..eafbe9e3bb4 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -863,11 +863,20 @@ static void dhcp_process_options(uchar *popt, uchar *end)
 #endif
 
 	while (popt < end && *popt != 0xff) {
+		if (*popt == 0) {
+			/* Pad option: single byte, no length field */
+			popt++;
+			continue;
+		}
+
+		/* Need a length byte, and the payload it describes */
+		if (popt + 1 >= end)
+			break;
 		oplen = *(popt + 1);
-		switch (*popt) {
-		case 0:
-			oplen = -1; /* Pad omits len byte */
+		if (popt + 2 + oplen > end)
 			break;
+
+		switch (*popt) {
 		case 1:
 			net_copy_ip(&net_netmask, (popt + 2));
 			break;
-- 
2.50.1 (Apple Git-155)

[PATCH v2 2/2] net: bootp: bound DHCP option parsing by the received packet length

[Pranav Rajendran]

dhcp_packet_process_options() derives the end of the option area from
BOOTP_HDR_SIZE, a compile-time constant, rather than from the length of
the packet that was actually received:

	uchar *popt = (uchar *)&bp->bp_vend[4];
	uchar *end = popt + BOOTP_HDR_SIZE;

Since popt already starts near the end of the header, 'end' lands
sizeof(struct bootp_hdr) bytes beyond it, so a short reply leaves
dhcp_process_options() walking off the end of the received data and
into whatever the receive buffer held before - typically the remains of
earlier packets.

That is not only a disclosure: the options found there are acted on
like any others, so stale bytes that happen to parse as an option can
influence the boot file name, the DNS server or the root path.

The BOOTP path already gets this right and passes the real length to
bootp_process_vendor(), and both callers here have the length in scope
- they hand it to dhcp_message_type() on the lines above. Pass it in
and use it as the limit. The overloaded 'file' and 'sname' areas are
clamped the same way, as a truncated packet need not contain them
either.

Fixes: 774c3e05ec0a ("net: parse DHCP options from overloaded file/sname fields")
Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
---
v2: No change, rebased on top of the new patch 1/2 which fixes the
    inner option-length validation gap Jerome raised against v1.

 net/bootp.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/net/bootp.c b/net/bootp.c
index eafbe9e3bb4..06083092875 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -977,32 +977,43 @@ static void dhcp_process_options(uchar *popt, uchar *end)
 	}
 }
 
-static void dhcp_packet_process_options(struct bootp_hdr *bp)
+static void dhcp_packet_process_options(struct bootp_hdr *bp, unsigned int len)
 {
-	uchar *popt = (uchar *)&bp->bp_vend[4];
-	uchar *end = popt + BOOTP_HDR_SIZE;
+	uchar *pkt_end = (uchar *)bp + len;
+	uchar *popt, *end;
+
+	if (len < offsetof(struct bootp_hdr, bp_vend) + 4)
+		return;
 
 	if (net_read_u32((u32 *)&bp->bp_vend[0]) != htonl(BOOTP_VENDOR_MAGIC))
 		return;
 
+	popt = (uchar *)&bp->bp_vend[4];
+
 	dhcp_option_overload = 0;
 
 	/*
 	 * The 'options' field MUST be interpreted first, 'file' next,
 	 * 'sname' last.
 	 */
-	dhcp_process_options(popt, end);
+	dhcp_process_options(popt, pkt_end);
 
 	if (dhcp_option_overload & OVERLOAD_FILE) {
 		popt = (uchar *)bp->bp_file;
 		end = popt + sizeof(bp->bp_file);
-		dhcp_process_options(popt, end);
+		if (end > pkt_end)
+			end = pkt_end;
+		if (popt < end)
+			dhcp_process_options(popt, end);
 	}
 
 	if (dhcp_option_overload & OVERLOAD_SNAME) {
 		popt = (uchar *)bp->bp_sname;
 		end = popt + sizeof(bp->bp_sname);
-		dhcp_process_options(popt, end);
+		if (end > pkt_end)
+			end = pkt_end;
+		if (popt < end)
+			dhcp_process_options(popt, end);
 	}
 }
 
@@ -1133,7 +1144,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 				debug("got BOOTP response; transitioning to BOUND\n");
 				goto dhcp_got_bootp;
 			}
-			dhcp_packet_process_options(bp);
+			dhcp_packet_process_options(bp, len);
 			if (CONFIG_IS_ENABLED(EFI_LOADER) &&
 			    IS_ENABLED(CONFIG_NETDEVICES))
 				efi_net_set_dhcp_ack(pkt, len);
@@ -1160,7 +1171,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 
 		if (dhcp_message_type((u8 *)bp->bp_vend, (u8 *)pkt + len) == DHCP_ACK) {
 dhcp_got_bootp:
-			dhcp_packet_process_options(bp);
+			dhcp_packet_process_options(bp, len);
 			/* Store net params from reply */
 			store_net_params(bp);
 			dhcp_state = BOUND;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2 2/2] net: bootp: bound DHCP option parsing by the received packet length

[Pranav R]

[-- Attachment #1: Type: text/plain, Size: 5763 bytes --]

Hi Jerome,

Good catch — you're right, the outer bound alone doesn't cover it.
dhcp_process_options() trusted `popt < end` to mean the whole option
(code + length + payload) was safe, but that only guarantees the code
byte is readable. oplen = *(popt + 1) and the payload dereferences in
the switch had no bound check of their own, so a packet truncated
right after an option code byte, or with a declared length that
overruns the buffer, still walked off the end even with the outer fix
applied.

Sent as v2 with a prerequisite patch (1/2) that fixes this: the pad
option is now handled before touching a second byte, a length byte is
required to exist before it's read, and the full declared option
(header + payload) is required to fit before the switch runs on it —
otherwise parsing stops instead of reading past the buffer. Your
original outer-bound fix is 2/2, unchanged, rebased on top.

Both commits are checkpatch --strict clean and build warning-free at
W=1; a full sandbox build also passes.

Thanks,
Pranav

On Thu, Aug 20, 2026 at 1:33 PM Pranav Rajendran <pranavkasthuri@gmail.com>
wrote:

> dhcp_packet_process_options() derives the end of the option area from
> BOOTP_HDR_SIZE, a compile-time constant, rather than from the length of
> the packet that was actually received:
>
>         uchar *popt = (uchar *)&bp->bp_vend[4];
>         uchar *end = popt + BOOTP_HDR_SIZE;
>
> Since popt already starts near the end of the header, 'end' lands
> sizeof(struct bootp_hdr) bytes beyond it, so a short reply leaves
> dhcp_process_options() walking off the end of the received data and
> into whatever the receive buffer held before - typically the remains of
> earlier packets.
>
> That is not only a disclosure: the options found there are acted on
> like any others, so stale bytes that happen to parse as an option can
> influence the boot file name, the DNS server or the root path.
>
> The BOOTP path already gets this right and passes the real length to
> bootp_process_vendor(), and both callers here have the length in scope
> - they hand it to dhcp_message_type() on the lines above. Pass it in
> and use it as the limit. The overloaded 'file' and 'sname' areas are
> clamped the same way, as a truncated packet need not contain them
> either.
>
> Fixes: 774c3e05ec0a ("net: parse DHCP options from overloaded file/sname
> fields")
> Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
> ---
> v2: No change, rebased on top of the new patch 1/2 which fixes the
>     inner option-length validation gap Jerome raised against v1.
>
>  net/bootp.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/net/bootp.c b/net/bootp.c
> index eafbe9e3bb4..06083092875 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -977,32 +977,43 @@ static void dhcp_process_options(uchar *popt, uchar
> *end)
>         }
>  }
>
> -static void dhcp_packet_process_options(struct bootp_hdr *bp)
> +static void dhcp_packet_process_options(struct bootp_hdr *bp, unsigned
> int len)
>  {
> -       uchar *popt = (uchar *)&bp->bp_vend[4];
> -       uchar *end = popt + BOOTP_HDR_SIZE;
> +       uchar *pkt_end = (uchar *)bp + len;
> +       uchar *popt, *end;
> +
> +       if (len < offsetof(struct bootp_hdr, bp_vend) + 4)
> +               return;
>
>         if (net_read_u32((u32 *)&bp->bp_vend[0]) !=
> htonl(BOOTP_VENDOR_MAGIC))
>                 return;
>
> +       popt = (uchar *)&bp->bp_vend[4];
> +
>         dhcp_option_overload = 0;
>
>         /*
>          * The 'options' field MUST be interpreted first, 'file' next,
>          * 'sname' last.
>          */
> -       dhcp_process_options(popt, end);
> +       dhcp_process_options(popt, pkt_end);
>
>         if (dhcp_option_overload & OVERLOAD_FILE) {
>                 popt = (uchar *)bp->bp_file;
>                 end = popt + sizeof(bp->bp_file);
> -               dhcp_process_options(popt, end);
> +               if (end > pkt_end)
> +                       end = pkt_end;
> +               if (popt < end)
> +                       dhcp_process_options(popt, end);
>         }
>
>         if (dhcp_option_overload & OVERLOAD_SNAME) {
>                 popt = (uchar *)bp->bp_sname;
>                 end = popt + sizeof(bp->bp_sname);
> -               dhcp_process_options(popt, end);
> +               if (end > pkt_end)
> +                       end = pkt_end;
> +               if (popt < end)
> +                       dhcp_process_options(popt, end);
>         }
>  }
>
> @@ -1133,7 +1144,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest,
> struct in_addr sip,
>                                 debug("got BOOTP response; transitioning
> to BOUND\n");
>                                 goto dhcp_got_bootp;
>                         }
> -                       dhcp_packet_process_options(bp);
> +                       dhcp_packet_process_options(bp, len);
>                         if (CONFIG_IS_ENABLED(EFI_LOADER) &&
>                             IS_ENABLED(CONFIG_NETDEVICES))
>                                 efi_net_set_dhcp_ack(pkt, len);
> @@ -1160,7 +1171,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest,
> struct in_addr sip,
>
>                 if (dhcp_message_type((u8 *)bp->bp_vend, (u8 *)pkt + len)
> == DHCP_ACK) {
>  dhcp_got_bootp:
> -                       dhcp_packet_process_options(bp);
> +                       dhcp_packet_process_options(bp, len);
>                         /* Store net params from reply */
>                         store_net_params(bp);
>                         dhcp_state = BOUND;
> --
> 2.50.1 (Apple Git-155)
>
>

[-- Attachment #2: Type: text/html, Size: 6964 bytes --]