public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] ipv6: validate extension header length before copying to cmsg
@ 2026-04-19 15:03 Qi Tang
  2026-04-23  8:45 ` Paolo Abeni
  2026-04-23 10:22 ` [PATCH net v2] " Qi Tang
  0 siblings, 2 replies; 6+ messages in thread
From: Qi Tang @ 2026-04-19 15:03 UTC (permalink / raw)
  To: davem, dsahern, edumazet, kuba, pabeni, horms
  Cc: netdev, linux-kernel, Qi Tang

ip6_datagram_recv_specific_ctl() builds IPV6_{HOPOPTS,DSTOPTS,RTHDR}
cmsgs (and their IPV6_2292* legacy counterparts) by trusting the
on-wire hdrlen byte (ptr[1]) when computing the put_cmsg() length.
The length was validated only at parse time (ipv6_parse_hopopts(),
etc.). An nftables payload-write expression can rewrite hdrlen after
parsing and before the skb reaches recvmsg; the write itself is
in-bounds but put_cmsg() then reads up to ((hdrlen+1) << 3) = 2040
bytes from an 8-byte header. nftables is reachable from an unprivi-
leged user namespace, so this is an unprivileged slab-out-of-bounds
read:

  BUG: KASAN: slab-out-of-bounds in put_cmsg+0x3ac/0x540
   put_cmsg+0x3ac/0x540
   udpv6_recvmsg+0xca0/0x1250
   sock_recvmsg+0xdf/0x190
   ____sys_recvmsg+0x1b1/0x620

Clamp each cmsg length against skb_tail_pointer(skb) before calling
put_cmsg(). Extension headers are kept in the linear skb area by
pskb_may_pull() during input, so skb_tail_pointer() is the correct
bound. The check is replicated at each call site (one HbH, four
RFC2292 sites, and four switch cases in the DSTOPTS/RTHDR/AH walk)
rather than hoisted out of the switch, to keep the fix minimal and
backportable; a follow-up cleanup can factor it out. In the walk
loop a failed check also aborts the walk, since subsequent offsets
depend on the tampered length.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Qi Tang <tpluszz77@gmail.com>
---
 net/ipv6/datagram.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index ca3605acb..a7b9f5a24 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -643,7 +643,10 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 	/* HbH is allowed only once */
 	if (np->rxopt.bits.hopopts && (opt->flags & IP6SKB_HOPBYHOP)) {
 		u8 *ptr = nh + sizeof(struct ipv6hdr);
-		put_cmsg(msg, SOL_IPV6, IPV6_HOPOPTS, (ptr[1]+1)<<3, ptr);
+		u16 hbhlen = (ptr[1] + 1) << 3;
+
+		if (ptr + hbhlen <= skb_tail_pointer(skb))
+			put_cmsg(msg, SOL_IPV6, IPV6_HOPOPTS, hbhlen, ptr);
 	}
 
 	if (opt->lastopt &&
@@ -668,27 +671,37 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 			case IPPROTO_DSTOPTS:
 				nexthdr = ptr[0];
 				len = (ptr[1] + 1) << 3;
+				if (ptr + len > skb_tail_pointer(skb))
+					goto ext_hdr_done;
 				if (np->rxopt.bits.dstopts)
 					put_cmsg(msg, SOL_IPV6, IPV6_DSTOPTS, len, ptr);
 				break;
 			case IPPROTO_ROUTING:
 				nexthdr = ptr[0];
 				len = (ptr[1] + 1) << 3;
+				if (ptr + len > skb_tail_pointer(skb))
+					goto ext_hdr_done;
 				if (np->rxopt.bits.srcrt)
 					put_cmsg(msg, SOL_IPV6, IPV6_RTHDR, len, ptr);
 				break;
 			case IPPROTO_AH:
 				nexthdr = ptr[0];
 				len = (ptr[1] + 2) << 2;
+				if (ptr + len > skb_tail_pointer(skb))
+					goto ext_hdr_done;
 				break;
 			default:
 				nexthdr = ptr[0];
 				len = (ptr[1] + 1) << 3;
+				if (ptr + len > skb_tail_pointer(skb))
+					goto ext_hdr_done;
 				break;
 			}
 
 			off += len;
 		}
+ext_hdr_done:
+		;
 	}
 
 	/* socket options in old style */
@@ -705,19 +718,31 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 	}
 	if (np->rxopt.bits.ohopopts && (opt->flags & IP6SKB_HOPBYHOP)) {
 		u8 *ptr = nh + sizeof(struct ipv6hdr);
-		put_cmsg(msg, SOL_IPV6, IPV6_2292HOPOPTS, (ptr[1]+1)<<3, ptr);
+		u16 hbhlen = (ptr[1] + 1) << 3;
+
+		if (ptr + hbhlen <= skb_tail_pointer(skb))
+			put_cmsg(msg, SOL_IPV6, IPV6_2292HOPOPTS, hbhlen, ptr);
 	}
 	if (np->rxopt.bits.odstopts && opt->dst0) {
 		u8 *ptr = nh + opt->dst0;
-		put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, (ptr[1]+1)<<3, ptr);
+		u16 doptlen = (ptr[1] + 1) << 3;
+
+		if (ptr + doptlen <= skb_tail_pointer(skb))
+			put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, doptlen, ptr);
 	}
 	if (np->rxopt.bits.osrcrt && opt->srcrt) {
 		struct ipv6_rt_hdr *rthdr = (struct ipv6_rt_hdr *)(nh + opt->srcrt);
-		put_cmsg(msg, SOL_IPV6, IPV6_2292RTHDR, (rthdr->hdrlen+1) << 3, rthdr);
+		u16 rtlen = (rthdr->hdrlen + 1) << 3;
+
+		if ((u8 *)rthdr + rtlen <= skb_tail_pointer(skb))
+			put_cmsg(msg, SOL_IPV6, IPV6_2292RTHDR, rtlen, rthdr);
 	}
 	if (np->rxopt.bits.odstopts && opt->dst1) {
 		u8 *ptr = nh + opt->dst1;
-		put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, (ptr[1]+1)<<3, ptr);
+		u16 doptlen = (ptr[1] + 1) << 3;
+
+		if (ptr + doptlen <= skb_tail_pointer(skb))
+			put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, doptlen, ptr);
 	}
 	if (np->rxopt.bits.rxorigdstaddr) {
 		struct sockaddr_in6 sin6;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] ipv6: validate extension header length before copying to cmsg
  2026-04-19 15:03 [PATCH net] ipv6: validate extension header length before copying to cmsg Qi Tang
@ 2026-04-23  8:45 ` Paolo Abeni
  2026-04-23  9:57   ` Qi Tang
  2026-04-23 10:22 ` [PATCH net v2] " Qi Tang
  1 sibling, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2026-04-23  8:45 UTC (permalink / raw)
  To: Qi Tang, davem, dsahern, edumazet, kuba, horms; +Cc: netdev, linux-kernel

On 4/19/26 5:03 PM, Qi Tang wrote:
> ip6_datagram_recv_specific_ctl() builds IPV6_{HOPOPTS,DSTOPTS,RTHDR}
> cmsgs (and their IPV6_2292* legacy counterparts) by trusting the
> on-wire hdrlen byte (ptr[1]) when computing the put_cmsg() length.
> The length was validated only at parse time (ipv6_parse_hopopts(),
> etc.). An nftables payload-write expression can rewrite hdrlen after
> parsing and before the skb reaches recvmsg; the write itself is
> in-bounds but put_cmsg() then reads up to ((hdrlen+1) << 3) = 2040
> bytes from an 8-byte header. nftables is reachable from an unprivi-
> leged user namespace, so this is an unprivileged slab-out-of-bounds
> read:
> 
>   BUG: KASAN: slab-out-of-bounds in put_cmsg+0x3ac/0x540
>    put_cmsg+0x3ac/0x540
>    udpv6_recvmsg+0xca0/0x1250
>    sock_recvmsg+0xdf/0x190
>    ____sys_recvmsg+0x1b1/0x620
> 
> Clamp each cmsg length against skb_tail_pointer(skb) before calling
> put_cmsg(). Extension headers are kept in the linear skb area by
> pskb_may_pull() during input, so skb_tail_pointer() is the correct
> bound. The check is replicated at each call site (one HbH, four
> RFC2292 sites, and four switch cases in the DSTOPTS/RTHDR/AH walk)
> rather than hoisted out of the switch, to keep the fix minimal and
> backportable; a follow-up cleanup can factor it out. In the walk
> loop a failed check also aborts the walk, since subsequent offsets
> depend on the tampered length.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Qi Tang <tpluszz77@gmail.com>
> ---
>  net/ipv6/datagram.c | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
> index ca3605acb..a7b9f5a24 100644
> --- a/net/ipv6/datagram.c
> +++ b/net/ipv6/datagram.c
> @@ -643,7 +643,10 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
>  	/* HbH is allowed only once */
>  	if (np->rxopt.bits.hopopts && (opt->flags & IP6SKB_HOPBYHOP)) {
>  		u8 *ptr = nh + sizeof(struct ipv6hdr);
> -		put_cmsg(msg, SOL_IPV6, IPV6_HOPOPTS, (ptr[1]+1)<<3, ptr);
> +		u16 hbhlen = (ptr[1] + 1) << 3;
> +
> +		if (ptr + hbhlen <= skb_tail_pointer(skb))
> +			put_cmsg(msg, SOL_IPV6, IPV6_HOPOPTS, hbhlen, ptr);

The patch looks functionally correct to me, but the above 3 statements
are repeated multiple times. You can put them in a local helper and
avoud a lot of duplicate code.

>  	}
>  
>  	if (opt->lastopt &&
> @@ -668,27 +671,37 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
>  			case IPPROTO_DSTOPTS:
>  				nexthdr = ptr[0];
>  				len = (ptr[1] + 1) << 3;
> +				if (ptr + len > skb_tail_pointer(skb))
> +					goto ext_hdr_done;

The packet is corrupted, allowing processing of later rxopt requires the
IMHO not nice empty label. I think it would be better just returning
from this function.

/P


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] ipv6: validate extension header length before copying to cmsg
  2026-04-23  8:45 ` Paolo Abeni
@ 2026-04-23  9:57   ` Qi Tang
  0 siblings, 0 replies; 6+ messages in thread
From: Qi Tang @ 2026-04-23  9:57 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
	Simon Horman, netdev, linux-kernel

On Thu, Apr 23, 2026, Paolo Abeni <pabeni@redhat.com> wrote:
> The patch looks functionally correct to me, but the above 3 statements
> are repeated multiple times. You can put them in a local helper and
> avoud a lot of duplicate code.

I actually considered that first, but went with per-site
replication in v1 to keep the diff minimal for backporting.
A local helper is definitely the better approach though.
Will do that for v2.

> The packet is corrupted, allowing processing of later rxopt requires the
> IMHO not nice empty label. I think it would be better just returning
> from this function.

Makes sense.  Will return directly in v2.

Thanks for the review.

Qi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net v2] ipv6: validate extension header length before copying to cmsg
  2026-04-19 15:03 [PATCH net] ipv6: validate extension header length before copying to cmsg Qi Tang
  2026-04-23  8:45 ` Paolo Abeni
@ 2026-04-23 10:22 ` Qi Tang
  2026-04-23 10:33   ` Qi Tang
  1 sibling, 1 reply; 6+ messages in thread
From: Qi Tang @ 2026-04-23 10:22 UTC (permalink / raw)
  To: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: netdev, linux-kernel

ip6_datagram_recv_specific_ctl() builds IPV6_{HOPOPTS,DSTOPTS,RTHDR}
cmsgs (and their IPV6_2292* legacy counterparts) by trusting the
on-wire hdrlen byte (ptr[1]) when computing the put_cmsg() length.
The length was validated only at parse time (ipv6_parse_hopopts(),
etc.).  An nftables payload-write expression can rewrite hdrlen after
parsing and before the skb reaches recvmsg; the write itself is
in-bounds but put_cmsg() then reads up to ((hdrlen+1) << 3) = 2040
bytes from an 8-byte header.  nftables is reachable from an
unprivileged user namespace, so this is an unprivileged
slab-out-of-bounds read:

  BUG: KASAN: slab-out-of-bounds in put_cmsg+0x3ac/0x540
   put_cmsg+0x3ac/0x540
   udpv6_recvmsg+0xca0/0x1250
   sock_recvmsg+0xdf/0x190
   ____sys_recvmsg+0x1b1/0x620

Add ipv6_get_exthdr_len() which computes the extension header length
and validates it against skb_tail_pointer(skb), returning 0 on
failure.  Extension headers are kept in the linear skb area by
pskb_may_pull() during input, so skb_tail_pointer() is the correct
bound.

Use ipv6_get_exthdr_len() at all non-AH call sites: the five
standalone cmsg blocks (HbH, 2292HbH, 2292DSTOPTS x2, 2292RTHDR)
and the three standard cases in the extension-header walk loop
(DSTOPTS, ROUTING, default).  AH retains an inline bounds check
because its length formula differs ((ptr[1]+2)<<2).

When the walk loop detects a corrupted header, return from the
function instead of continuing to process later socket options.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Qi Tang <tpluszz77@gmail.com>
---
Changes v1 -> v2 (Paolo Abeni):
  - Factor repeated bounds-check + put_cmsg into ipv6_get_exthdr_len()
  - Return from the function on corrupted walk-loop entry instead of
    goto + empty label

v1: https://lore.kernel.org/netdev/20260419150344.624673-1-tpluszz77@gmail.com/

 net/ipv6/datagram.c | 46 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 972bf0426d59..0a7b74d5f402 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -617,6 +617,13 @@ void ip6_datagram_recv_common_ctl(struct sock *sk, struct msghdr *msg,
 	}
 }
 
+static u16 ipv6_get_exthdr_len(const struct sk_buff *skb, const u8 *ptr)
+{
+	u16 len = (ptr[1] + 1) << 3;
+
+	return (ptr + len <= skb_tail_pointer(skb)) ? len : 0;
+}
+
 void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 				    struct sk_buff *skb)
 {
@@ -643,7 +650,10 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 	/* HbH is allowed only once */
 	if (np->rxopt.bits.hopopts && (opt->flags & IP6SKB_HOPBYHOP)) {
 		u8 *ptr = nh + sizeof(struct ipv6hdr);
-		put_cmsg(msg, SOL_IPV6, IPV6_HOPOPTS, (ptr[1]+1)<<3, ptr);
+		u16 len = ipv6_get_exthdr_len(skb, ptr);
+
+		if (len)
+			put_cmsg(msg, SOL_IPV6, IPV6_HOPOPTS, len, ptr);
 	}
 
 	if (opt->lastopt &&
@@ -667,23 +677,31 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 			switch (nexthdr) {
 			case IPPROTO_DSTOPTS:
 				nexthdr = ptr[0];
-				len = (ptr[1] + 1) << 3;
+				len = ipv6_get_exthdr_len(skb, ptr);
+				if (!len)
+					return;
 				if (np->rxopt.bits.dstopts)
 					put_cmsg(msg, SOL_IPV6, IPV6_DSTOPTS, len, ptr);
 				break;
 			case IPPROTO_ROUTING:
 				nexthdr = ptr[0];
-				len = (ptr[1] + 1) << 3;
+				len = ipv6_get_exthdr_len(skb, ptr);
+				if (!len)
+					return;
 				if (np->rxopt.bits.srcrt)
 					put_cmsg(msg, SOL_IPV6, IPV6_RTHDR, len, ptr);
 				break;
 			case IPPROTO_AH:
 				nexthdr = ptr[0];
 				len = (ptr[1] + 2) << 2;
+				if (ptr + len > skb_tail_pointer(skb))
+					return;
 				break;
 			default:
 				nexthdr = ptr[0];
-				len = (ptr[1] + 1) << 3;
+				len = ipv6_get_exthdr_len(skb, ptr);
+				if (!len)
+					return;
 				break;
 			}
 
@@ -705,19 +723,31 @@ void ip6_datagram_recv_specific_ctl(struct sock *sk, struct msghdr *msg,
 	}
 	if (np->rxopt.bits.ohopopts && (opt->flags & IP6SKB_HOPBYHOP)) {
 		u8 *ptr = nh + sizeof(struct ipv6hdr);
-		put_cmsg(msg, SOL_IPV6, IPV6_2292HOPOPTS, (ptr[1]+1)<<3, ptr);
+		u16 len = ipv6_get_exthdr_len(skb, ptr);
+
+		if (len)
+			put_cmsg(msg, SOL_IPV6, IPV6_2292HOPOPTS, len, ptr);
 	}
 	if (np->rxopt.bits.odstopts && opt->dst0) {
 		u8 *ptr = nh + opt->dst0;
-		put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, (ptr[1]+1)<<3, ptr);
+		u16 len = ipv6_get_exthdr_len(skb, ptr);
+
+		if (len)
+			put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, len, ptr);
 	}
 	if (np->rxopt.bits.osrcrt && opt->srcrt) {
 		struct ipv6_rt_hdr *rthdr = (struct ipv6_rt_hdr *)(nh + opt->srcrt);
-		put_cmsg(msg, SOL_IPV6, IPV6_2292RTHDR, (rthdr->hdrlen+1) << 3, rthdr);
+		u16 len = ipv6_get_exthdr_len(skb, (u8 *)rthdr);
+
+		if (len)
+			put_cmsg(msg, SOL_IPV6, IPV6_2292RTHDR, len, rthdr);
 	}
 	if (np->rxopt.bits.odstopts && opt->dst1) {
 		u8 *ptr = nh + opt->dst1;
-		put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, (ptr[1]+1)<<3, ptr);
+		u16 len = ipv6_get_exthdr_len(skb, ptr);
+
+		if (len)
+			put_cmsg(msg, SOL_IPV6, IPV6_2292DSTOPTS, len, ptr);
 	}
 	if (np->rxopt.bits.rxorigdstaddr) {
 		struct sockaddr_in6 sin6;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] ipv6: validate extension header length before copying to cmsg
  2026-04-23 10:22 ` [PATCH net v2] " Qi Tang
@ 2026-04-23 10:33   ` Qi Tang
  2026-04-23 10:58     ` Paolo Abeni
  0 siblings, 1 reply; 6+ messages in thread
From: Qi Tang @ 2026-04-23 10:33 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
	Simon Horman, netdev, linux-kernel, Qi Tang

Please drop v2 — it was incorrectly sent as a reply to v1 instead
of a new thread.  Resent as v3:
https://lore.kernel.org/netdev/20260423103238.3987364-1-tpluszz77@gmail.com/

Sorry for the noise.

Qi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] ipv6: validate extension header length before copying to cmsg
  2026-04-23 10:33   ` Qi Tang
@ 2026-04-23 10:58     ` Paolo Abeni
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2026-04-23 10:58 UTC (permalink / raw)
  To: Qi Tang
  Cc: David S . Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
	Simon Horman, netdev, linux-kernel

On 4/23/26 12:33 PM, Qi Tang wrote:
> Please drop v2 — it was incorrectly sent as a reply to v1 instead
> of a new thread.  Resent as v3:
> https://lore.kernel.org/netdev/20260423103238.3987364-1-tpluszz77@gmail.com/

FTR, you should have waited 24h before sending v3. Beyond keeping the
traffic on the ML under control, that helps you learning and remembering
the process.

/P


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-23 10:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-19 15:03 [PATCH net] ipv6: validate extension header length before copying to cmsg Qi Tang
2026-04-23  8:45 ` Paolo Abeni
2026-04-23  9:57   ` Qi Tang
2026-04-23 10:22 ` [PATCH net v2] " Qi Tang
2026-04-23 10:33   ` Qi Tang
2026-04-23 10:58     ` Paolo Abeni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox