[v4] bpf: tcp: Fix null-ptr-deref in arbitrary SYN Cookie

[PATCH bpf v4 1/2] bpf: tcp: Reject non-TCP skb in bpf_sk_assign_tcp_reqsk()

Posted by Jiayuan Chen 1 week ago

bpf_sk_assign_tcp_reqsk() only validates skb->protocol (L3) but does not
check the L4 protocol in the IP header. A BPF program can call this kfunc
on a UDP skb with a valid TCP listener socket, which will succeed and
attach a TCP reqsk to the UDP skb.

When the UDP skb enters the UDP receive path, skb_steal_sock() returns
the TCP listener from the reqsk. The UDP code then passes this TCP socket
to udp_unicast_rcv_skb() -> __udp_enqueue_schedule_skb(), which casts
it to udp_sock and accesses UDP-specific fields at invalid offsets,
causing a null pointer dereference and kernel panic:

  BUG: KASAN: null-ptr-deref in __udp_enqueue_schedule_skb+0x19d/0x1df0
  Read of size 4 at addr 0000000000000008 by task test_progs/537

  CPU: 1 UID: 0 PID: 537 Comm: test_progs Not tainted 7.0.0-rc4+ #46 PREEMPT
  Call Trace:
   <IRQ>
   dump_stack_lvl (lib/dump_stack.c:123)
   print_report (mm/kasan/report.c:487)
   kasan_report (mm/kasan/report.c:597)
   __kasan_check_read (mm/kasan/shadow.c:32)
   __udp_enqueue_schedule_skb (net/ipv4/udp.c:1719)
   udp_queue_rcv_one_skb (net/ipv4/udp.c:2370 net/ipv4/udp.c:2500)
   udp_queue_rcv_skb (net/ipv4/udp.c:2532)
   udp_unicast_rcv_skb (net/ipv4/udp.c:2684)
   __udp4_lib_rcv (net/ipv4/udp.c:2742)
   udp_rcv (net/ipv4/udp.c:2937)
   ip_protocol_deliver_rcu (net/ipv4/ip_input.c:209)
   ip_local_deliver_finish (./include/linux/rcupdate.h:879 net/ipv4/ip_input.c:242)
   ip_local_deliver (net/ipv4/ip_input.c:265)
   __netif_receive_skb_one_core (net/core/dev.c:6164 (discriminator 4))
   __netif_receive_skb (net/core/dev.c:6280)

Fix this by checking the IP header's protocol field in
bpf_sk_assign_tcp_reqsk() and rejecting non-TCP skbs with -EINVAL.

Fixes: e472f88891ab ("bpf: tcp: Support arbitrary SYN Cookie.")
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/core/filter.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 78b548158fb0..fb975bcce804 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -12248,11 +12248,17 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk,
 
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
+		if (ip_hdr(skb)->protocol != IPPROTO_TCP)
+			return -EINVAL;
+
 		ops = &tcp_request_sock_ops;
 		min_mss = 536;
 		break;
 #if IS_BUILTIN(CONFIG_IPV6)
 	case htons(ETH_P_IPV6):
+		if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)
+			return -EINVAL;
+
 		ops = &tcp6_request_sock_ops;
 		min_mss = IPV6_MIN_MTU - 60;
 		break;
-- 
2.43.0

Re: [PATCH bpf v4 1/2] bpf: tcp: Reject non-TCP skb in bpf_sk_assign_tcp_reqsk()

Posted by bot+bpf-ci@kernel.org 1 week ago

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 78b548158fb0..fb975bcce804 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -12248,11 +12248,17 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk,
>
>  	switch (skb->protocol) {

[ ... ]

> #if IS_BUILTIN(CONFIG_IPV6)
>  	case htons(ETH_P_IPV6):
> +		if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)
> +			return -EINVAL;
> +
>  		ops = &tcp6_request_sock_ops;

For IPv6, ipv6_hdr(skb)->nexthdr gives the type of the header
immediately following the base IPv6 header. If extension headers
are present (hop-by-hop options, routing, etc.), nexthdr would be
the extension header type rather than IPPROTO_TCP, even when the
packet is a valid TCP segment.

Would it be worth using ipv6_find_hdr() here, similar to
bpf_update_srh_state() in the same file, to walk past any extension
headers? The IPv4 check above is fine since ip_hdr(skb)->protocol
always identifies the transport protocol directly.

In practice this is very unlikely to matter since TCP SYN packets
with IPv6 extension headers are essentially non-existent, but the
check as written would reject them.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23735111188

Re: [PATCH bpf v4 1/2] bpf: tcp: Reject non-TCP skb in bpf_sk_assign_tcp_reqsk()

Posted by Jiayuan Chen 6 days, 23 hours ago

On 3/30/26 5:00 PM, bot+bpf-ci@kernel.org wrote:
>> #if IS_BUILTIN(CONFIG_IPV6)
>>   	case htons(ETH_P_IPV6):
>> +		if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)
>> +			return -EINVAL;
>> +
>>   		ops = &tcp6_request_sock_ops;
> For IPv6, ipv6_hdr(skb)->nexthdr gives the type of the header
> immediately following the base IPv6 header. If extension headers
> are present (hop-by-hop options, routing, etc.), nexthdr would be
> the extension header type rather than IPPROTO_TCP, even when the
> packet is a valid TCP segment.
>
> Would it be worth using ipv6_find_hdr() here, similar to
> bpf_update_srh_state() in the same file, to walk past any extension
> headers? The IPv4 check above is fine since ip_hdr(skb)->protocol
> always identifies the transport protocol directly.
>
> In practice this is very unlikely to matter since TCP SYN packets
> with IPv6 extension headers are essentially non-existent, but the
> check as written would reject them.
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See:https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary:https://github.com/kernel-patches/bpf/actions/runs/23735111188


Thanks for the analysis.

There are many places in the kernel that check nexthdr directly without
walking extension headers.

I'd prefer to keep it simple for now and leave ipv6_find_hdr() as a
potential future improvement if needed.

But happy to change if reviewersthink otherwise.

Re: [PATCH bpf v4 1/2] bpf: tcp: Reject non-TCP skb in bpf_sk_assign_tcp_reqsk()

Posted by Kuniyuki Iwashima 6 days, 7 hours ago

On Mon, Mar 30, 2026 at 2:26 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>
> On 3/30/26 5:00 PM, bot+bpf-ci@kernel.org wrote:
> >> #if IS_BUILTIN(CONFIG_IPV6)
> >>      case htons(ETH_P_IPV6):
> >> +            if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)
> >> +                    return -EINVAL;
> >> +
> >>              ops = &tcp6_request_sock_ops;
> > For IPv6, ipv6_hdr(skb)->nexthdr gives the type of the header
> > immediately following the base IPv6 header. If extension headers
> > are present (hop-by-hop options, routing, etc.), nexthdr would be
> > the extension header type rather than IPPROTO_TCP, even when the
> > packet is a valid TCP segment.
> >
> > Would it be worth using ipv6_find_hdr() here, similar to
> > bpf_update_srh_state() in the same file, to walk past any extension
> > headers? The IPv4 check above is fine since ip_hdr(skb)->protocol
> > always identifies the transport protocol directly.
> >
> > In practice this is very unlikely to matter since TCP SYN packets
> > with IPv6 extension headers are essentially non-existent, but the
> > check as written would reject them.
> >
> >
> > ---
> > AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> > See:https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> >
> > CI run summary:https://github.com/kernel-patches/bpf/actions/runs/23735111188
>
>
> Thanks for the analysis.
>
> There are many places in the kernel that check nexthdr directly without
> walking extension headers.
>
> I'd prefer to keep it simple for now and leave ipv6_find_hdr() as a
> potential future improvement if needed.

+1.

Given this feature is to create a reqsk to process on this running
host, it does not make sense to support such ext options.

Re: [PATCH bpf v4 1/2] bpf: tcp: Reject non-TCP skb in bpf_sk_assign_tcp_reqsk()

Posted by Martin KaFai Lau 6 days, 5 hours ago

On 3/30/26 6:35 PM, Kuniyuki Iwashima wrote:
> On Mon, Mar 30, 2026 at 2:26 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>>
>>
>> On 3/30/26 5:00 PM, bot+bpf-ci@kernel.org wrote:
>>>> #if IS_BUILTIN(CONFIG_IPV6)
>>>>       case htons(ETH_P_IPV6):
>>>> +            if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)
>>>> +                    return -EINVAL;
>>>> +
>>>>               ops = &tcp6_request_sock_ops;
>>> For IPv6, ipv6_hdr(skb)->nexthdr gives the type of the header
>>> immediately following the base IPv6 header. If extension headers
>>> are present (hop-by-hop options, routing, etc.), nexthdr would be
>>> the extension header type rather than IPPROTO_TCP, even when the
>>> packet is a valid TCP segment.
>>>
>>> Would it be worth using ipv6_find_hdr() here, similar to
>>> bpf_update_srh_state() in the same file, to walk past any extension
>>> headers? The IPv4 check above is fine since ip_hdr(skb)->protocol
>>> always identifies the transport protocol directly.
>>>
>>> In practice this is very unlikely to matter since TCP SYN packets
>>> with IPv6 extension headers are essentially non-existent, but the
>>> check as written would reject them.
>>>
>>>
>>> ---
>>> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
>>> See:https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>>>
>>> CI run summary:https://github.com/kernel-patches/bpf/actions/runs/23735111188
>>
>>
>> Thanks for the analysis.
>>
>> There are many places in the kernel that check nexthdr directly without
>> walking extension headers.
>>
>> I'd prefer to keep it simple for now and leave ipv6_find_hdr() as a
>> potential future improvement if needed.
> 
> +1.
> 
> Given this feature is to create a reqsk to process on this running
> host, it does not make sense to support such ext options.

While at header reading, does it need a pskb_may_pull() before reading?

Re: [PATCH bpf v4 1/2] bpf: tcp: Reject non-TCP skb in bpf_sk_assign_tcp_reqsk()

Posted by Kuniyuki Iwashima 5 days, 11 hours ago

On Mon, Mar 30, 2026 at 8:19 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 3/30/26 6:35 PM, Kuniyuki Iwashima wrote:
> > On Mon, Mar 30, 2026 at 2:26 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> >>
> >>
> >> On 3/30/26 5:00 PM, bot+bpf-ci@kernel.org wrote:
> >>>> #if IS_BUILTIN(CONFIG_IPV6)
> >>>>       case htons(ETH_P_IPV6):
> >>>> +            if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)
> >>>> +                    return -EINVAL;
> >>>> +
> >>>>               ops = &tcp6_request_sock_ops;
> >>> For IPv6, ipv6_hdr(skb)->nexthdr gives the type of the header
> >>> immediately following the base IPv6 header. If extension headers
> >>> are present (hop-by-hop options, routing, etc.), nexthdr would be
> >>> the extension header type rather than IPPROTO_TCP, even when the
> >>> packet is a valid TCP segment.
> >>>
> >>> Would it be worth using ipv6_find_hdr() here, similar to
> >>> bpf_update_srh_state() in the same file, to walk past any extension
> >>> headers? The IPv4 check above is fine since ip_hdr(skb)->protocol
> >>> always identifies the transport protocol directly.
> >>>
> >>> In practice this is very unlikely to matter since TCP SYN packets
> >>> with IPv6 extension headers are essentially non-existent, but the
> >>> check as written would reject them.
> >>>
> >>>
> >>> ---
> >>> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> >>> See:https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> >>>
> >>> CI run summary:https://github.com/kernel-patches/bpf/actions/runs/23735111188
> >>
> >>
> >> Thanks for the analysis.
> >>
> >> There are many places in the kernel that check nexthdr directly without
> >> walking extension headers.
> >>
> >> I'd prefer to keep it simple for now and leave ipv6_find_hdr() as a
> >> potential future improvement if needed.
> >
> > +1.
> >
> > Given this feature is to create a reqsk to process on this running
> > host, it does not make sense to support such ext options.
>
> While at header reading, does it need a pskb_may_pull() before reading?

Ah good point, now that we read skb->data, pskb_may_pull() is needed indeed.