Re: [PATCH net] net/ipv6: don't route packets with unknown source address

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] tcp_v6_send_response() calls inet_sk_flowi_flags() on TIME_WAIT sockets, resulting in an out-of-bounds memory read.
- [High] ip6_route_me_harder() incorrectly checks the destination address type instead of the source address type, zeroing the source address for multicast packets and bypassing security checks for unicast.
- [High] IPv6 SCTP transparent sockets are broken because FLOWI_FLAG_ANYSRC is not set during route lookup.
- [High] tcp_v6_connect() overwrites flowi6_flags, inadvertently dropping FLOWI_FLAG_ANYSRC for transparent sockets when multipath routing is enabled.
--

commit 4450c087e2afc1b6d2ec6336ee396246ee37e850
Author: Íñigo Huguet <ihuguet@riseup.net>

    net/ipv6: don't route packets with unknown source address
    
    Don't allow routing packets with a source address that is not configured
    in the host. Allow it only in certain cases like when using a
    transparent socket, by setting the ANYSRC flag in flowi_flags.

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 8fc4766c8da90..57eebe347902b 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1191,6 +1191,34 @@ static int ip6_dst_lookup_tail(struct net *net, const struct sock *sk,
>  	if (err)
>  		goto out_err_release;
>  
> +	/* Routing packets with a source address not present in the host is
> +	 * disallowed unless the ANYSRC flag is set (i.e. with transparent sockets).
> +	 * The source address must be in the same L3 domain as the destination device.
> +	 */
> +	if (!ipv6_addr_any(&fl6->saddr) &&
> +	    !(fl6->flowi6_flags & FLOWI_FLAG_ANYSRC)) {

[Severity: High]
Will this break IPv6 SCTP transparent sockets?

SCTP sockets using IP_TRANSPARENT with a non-local source address fail to route
packets because sctp_v6_get_dst() initializes flowi6 but does not set
flowi6_flags using inet_sk_flowi_flags(sk). As a result, the check here
rejects the packet due to the missing FLOWI_FLAG_ANYSRC flag.

[ ... ]

> diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
> index a7025ec870359..7c19ecbec112e 100644
> --- a/net/ipv6/netfilter.c
> +++ b/net/ipv6/netfilter.c
> @@ -41,6 +41,12 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
>  	};
>  	int err;
>  
> +	fl6.flowi6_flags = sk ? inet_sk_flowi_flags(sk) : 0;
> +	if (daddr_type & (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL))
> +		fl6.flowi6_flags |= FLOWI_FLAG_ANYSRC;
> +	else
> +		fl6.saddr = in6addr_any;

[Severity: High]
Does this logic mistakenly check the destination address type instead of the
source address?

Checking daddr_type here causes the source address to be zeroed out for
multicast traffic, breaking multicast routing. It also bypasses the newly
added anti-spoofing checks for unicast traffic.

[ ... ]

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index df9c29eb5c1f4..401a1297e7dba 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -268,6 +268,7 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
>  	fl6->flowi6_mark = sk->sk_mark;
>  	fl6->fl6_dport = usin->sin6_port;
>  	fl6->fl6_sport = inet->inet_sport;
> +	fl6->flowi6_flags = inet_sk_flowi_flags(sk);
>  	if (IS_ENABLED(CONFIG_IP_ROUTE_MULTIPATH) && !fl6->fl6_sport)
>  		fl6->flowi6_flags = FLOWI_FLAG_ANY_SPORT;

[Severity: High]
Is it intended to overwrite flowi6_flags and drop FLOWI_FLAG_ANYSRC?

If multipath routing is enabled (CONFIG_IP_ROUTE_MULTIPATH) and no source
port is bound, FLOWI_FLAG_ANY_SPORT is assigned using = instead of |=.
This drops the FLOWI_FLAG_ANYSRC flag, causing transparent sockets to fail
routing when attempting to connect.

[ ... ]

> @@ -979,6 +980,7 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
>  	fl6.fl6_dport = t1->dest;
>  	fl6.fl6_sport = t1->source;
>  	fl6.flowi6_uid = sock_net_uid(net, sk && sk_fullsock(sk) ? sk : NULL);
> +	fl6.flowi6_flags = sk ? inet_sk_flowi_flags(sk) : 0;

[Severity: Critical]
Can this result in an out-of-bounds memory read?

Unlike the previous line which explicitly checks sk_fullsock(sk), this calls
inet_sk_flowi_flags(sk) directly. tcp_v6_send_response() can be invoked
with sk pointing to an inet_timewait_sock. inet_sk_flowi_flags() uses
inet_test_bit(TRANSPARENT, sk), casting the socket to struct inet_sock.
Since inet_timewait_sock is smaller than inet_sock, this reads past the
end of the socket allocation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901115021.50057-1-ihuguet@riseup.net?part=1