Re: [PATCH v2 bpf 1/6] bpf: tcp: Fix type confusion in bpf_tcp_sock().

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The OpenVPN (ovpn) driver contains a critical type confusion vulnerability due to insufficient socket type checking, leading to out-of-bounds slab writes.
--

commit 3e93f247feecc891b15b8d713331586af9371dfd
Author: Kuniyuki Iwashima <kuniyu@google.com>

bpf: tcp: Fix type confusion in bpf_tcp_sock().

This commit fixes an out-of-bounds slab access bug in BPF by using sk_is_tcp()
instead of just checking sk_protocol for IPPROTO_TCP. This prevents raw sockets
created with IPPROTO_TCP from bypassing the type check.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index bc96c18df4e03..cd88633f8dc14 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7475,7 +7475,7 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
>  
>  BPF_CALL_1(bpf_tcp_sock, struct sock *, sk)
>  {
> -	if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP)
> +	if (sk_fullsock(sk) && sk_is_tcp(sk))

This correctly prevents a type confusion where a raw socket can bypass the
protocol check. Does the OpenVPN (ovpn) driver need a similar fix for this
exact same pattern?

In drivers/net/ovpn/netlink.c, the driver accepts an arbitrary socket file
descriptor from userspace and determines the socket type using only sk_protocol
without verifying sk_type.

If a user creates a raw socket via socket(AF_INET, SOCK_RAW, IPPROTO_UDP) and
passes it to the OpenVPN netlink interface, it seems it would pass the protocol
check and call ovpn_udp_socket_attach().

This eventually passes the socket to setup_udp_tunnel_sock(), which casts the
socket to udp_sk(sk) and overwrites multiple fields (such as encap_type,
encap_rcv, and gro_receive).

Since struct udp_sock is larger than struct raw_sock, could this result in an
out-of-bounds slab write?

Similarly, passing a raw TCP socket appears to cause ovpn_tcp_socket_attach()
to replace sk->sk_prot with ovpn_tcp_prot. This might mean subsequent TCP
socket operations will operate out-of-bounds on the raw_sock object.

Should ovpn be updated to validate sk_type == SOCK_DGRAM or SOCK_STREAM in a
similar manner to this fix?

>  		return (unsigned long)sk;
>  
>  	return (unsigned long)NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504210610.180150-1-kuniyu@google.com?part=1