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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Link-Local Multicast addresses bypass source address clearing in `ip6_route_me_harder`.
- [High] TCP resets and ACKs from IPv6 transparent request (NEW_SYN_RECV) and TIME_WAIT sockets are dropped due to missing FLOWI_FLAG_ANYSRC.
- [High] SCTP IPv6 transparent sockets are broken because `FLOWI_FLAG_ANYSRC` is not set in `sctp_v6_get_dst`.
--

commit 0afe460974f748f250f2af7329befb04fd3c1e35
Author: Íñigo Huguet <ihuguet@riseup.net>
Subject: net/ipv6: don't route packets with unknown source address

This commit prevents outbound IPv6 packets from being routed if their source
address is not configured on the host, aligning the behavior with the IPv4
stack. It adds source address validation to ip6_dst_lookup_tail and applies
the FLOWI_FLAG_ANYSRC flag in various protocol handlers to allow certain
cases like transparent sockets.

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 8fc4766c8da90..a83ca449e3089 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 src addr not present in the host is not
> +	 * allowed unless the ANYSRC flag is set (i.e. with transparent socket).
> +	 * The address must be in the same L3 domain as the destination device.
> +	 */
> +	if (!ipv6_addr_any(&fl6->saddr) &&
> +	    !(fl6->flowi6_flags & FLOWI_FLAG_ANYSRC)) {
> +		struct net_device *oif_dev;

[Severity: High]
Does this new validation block break SCTP IPv6 transparent sockets?

In net/sctp/ipv6.c, sctp_v6_get_dst() calls ip6_dst_lookup_flow() but
was not updated in this patch to set fl6.flowi6_flags = inet_sk_flowi_flags(sk).

Since this new check blocks non-local source addresses unless FLOWI_FLAG_ANYSRC
is set, will a transparent SCTP socket attempting to route a packet with a
foreign source address now be incorrectly rejected here?

> +
> +		rcu_read_lock();
> +		/* For local routes (*dst)->dev can be 'lo', which has no l3mdev
> +		 * master, so that the L3 domain wouldn't match if the source
> +		 * address is in a VRF-enslaved device. To avoid that, check the
> +		 * outgoing interface from the flowi6 structure instead.
> +		 */
> +		if (fl6->flowi6_oif)
> +			oif_dev = dev_get_by_index_rcu(net, fl6->flowi6_oif);
> +		else
> +			oif_dev = (*dst)->dev;
> +
> +		if (!ipv6_chk_addr_and_flags(net, &fl6->saddr, oif_dev,
> +					     1, 1, IFA_F_TENTATIVE))
> +			err = -ENETUNREACH;
> +		rcu_read_unlock();
> +
> +		if (err)
> +			goto out_err_release;
> +	}
> +
[ ... ]
> diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
> index a7025ec870359..716fc6e246de9 100644
> --- a/net/ipv6/netfilter.c
> +++ b/net/ipv6/netfilter.c
[ ... ]
> @@ -41,6 +42,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 (saddr_type & (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL))
> +		fl6.flowi6_flags |= FLOWI_FLAG_ANYSRC;
> +	else
> +		fl6.saddr = in6addr_any;

[Severity: High]
Does this logic incorrectly evaluate to true for Link-Local Multicast
addresses?

If ipv6_addr_type() returns IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL
for an address like ff02::1, the bitmask check:

saddr_type & (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)

will evaluate to true due to the IPV6_ADDR_LINKLOCAL bit being set.

Will this mistakenly apply FLOWI_FLAG_ANYSRC to link-local multicast
addresses, bypassing the in6addr_any clearing intended to sanitize
invalid source IPs?

> +
>  	if (sk && sk->sk_bound_dev_if)
>  		fl6.flowi6_oif = sk->sk_bound_dev_if;
>  	else if (strict)
[ ... ]
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index df9c29eb5c1f4..ae69e71b08598 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
[ ... ]
> @@ -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 && sk_fullsock(sk) ? inet_sk_flowi_flags(sk) : 0;

[Severity: High]
Can this inadvertently drop TCP resets and ACKs from IPv6 transparent
sockets in TIME_WAIT or NEW_SYN_RECV states?

When tcp_v6_send_response() is called for these states, sk_fullsock(sk)
evaluates to false. As a result, FLOWI_FLAG_ANYSRC is not applied.

Later, when ip6_dst_lookup_tail() validates the transparent (foreign)
source address without the ANYSRC flag, won't it reject the packet and
return -ENETUNREACH, breaking connection teardown and SYN-ACK responses?

>  	security_skb_classify_flow(skb, flowi6_to_flowi_common(&fl6));
>  
>  	/* Pass a socket to ip6_dst_lookup either it is for RST

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