[PATCH net] ipv6: fix cached route reuse for SO_MARK control messages

[PATCH net] ipv6: fix cached route reuse for SO_MARK control messages

[Xia Yan]

A connected IPv6 UDP socket can reuse its cached route when a SO_MARK
control message supplies a different mark. The packet then takes the
route selected for the socket mark instead of the per-message mark.

udpv6_sendmsg() sets flowi6_mark from the control message and clears
connected. However, ip6_sk_dst_lookup_flow() still tries the cached dst;
connected only controls whether a newly looked-up route is stored.
ip6_sk_dst_check() checks the addresses and output interface, but not
the mark.

Commit 42dcfd850e51 ("udp6: allow SO_MARK ctrl msg to affect routing")
fixed the mark used for fresh route lookups; connected sockets still
reuse the route cached before the control message.

Reject the cached dst when flowi6_mark differs from sk_mark so that
the packet goes through a new route lookup with the requested mark.

Found during code review of IPv6 UDP route caching. Reproduced in UML
with a connected socket and a fwmark rule pointing to a prohibit route:
marked sends succeeded before the fix and return EACCES after it.
A two-interface test also confirms that marked sends use the selected
interface and subsequent sends without a control message still use the
socket mark. cmsg_so_mark.sh passes before and after.

Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
Cc: stable@vger.kernel.org
Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>
---

Tested on net 5838f10563d58afca6711edb5e4eda968ca181d9. The relevant
send and route-cache code is unchanged at the base commit.

Full x86_64 W=1 builds completed before and after with no new
diagnostics, using configurations based on allmodconfig and
allyesconfig. WERROR was disabled for both; allyesconfig also had
KASAN disabled. Both used LD_ORPHAN_WARN_LEVEL="warn" and
DRM_WERROR=n.

 net/ipv6/ip6_output.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 25fe0ba98b1a6..843bb95e635b9 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1125,6 +1125,7 @@ static struct dst_entry *ip6_sk_dst_check(struct sock *sk,
 	    ip6_rt_check(&rt->rt6i_src, &fl6->saddr,
 			 np->saddr_cache ? &np->saddr : NULL) ||
 #endif
+	    fl6->flowi6_mark != READ_ONCE(sk->sk_mark) ||
 	   (fl6->flowi6_oif && fl6->flowi6_oif != dst_dev(dst)->ifindex)) {
 		dst_release(dst);
 		dst = NULL;

base-commit: d8a4f4c59da52b7c31ae337243b8473a19a3f819
-- 
2.39.0.windows.2

Re: [PATCH net] ipv6: fix cached route reuse for SO_MARK control messages

[Willem de Bruijn]

Xia Yan wrote:
> A connected IPv6 UDP socket can reuse its cached route when a SO_MARK
> control message supplies a different mark. The packet then takes the
> route selected for the socket mark instead of the per-message mark.
> 
> udpv6_sendmsg() sets flowi6_mark from the control message and clears
> connected. However, ip6_sk_dst_lookup_flow() still tries the cached dst;
> connected only controls whether a newly looked-up route is stored.
> ip6_sk_dst_check() checks the addresses and output interface, but not
> the mark.
> 
> Commit 42dcfd850e51 ("udp6: allow SO_MARK ctrl msg to affect routing")
> fixed the mark used for fresh route lookups; connected sockets still
> reuse the route cached before the control message.
> 
> Reject the cached dst when flowi6_mark differs from sk_mark so that
> the packet goes through a new route lookup with the requested mark.
> 
> Found during code review of IPv6 UDP route caching. Reproduced in UML
> with a connected socket and a fwmark rule pointing to a prohibit route:
> marked sends succeeded before the fix and return EACCES after it.
> A two-interface test also confirms that marked sends use the selected
> interface and subsequent sends without a control message still use the
> socket mark. cmsg_so_mark.sh passes before and after.
> 
> Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xia Yan <yanxia@cmss.chinamobile.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> ---
> 
> Tested on net 5838f10563d58afca6711edb5e4eda968ca181d9. The relevant
> send and route-cache code is unchanged at the base commit.
> 
> Full x86_64 W=1 builds completed before and after with no new
> diagnostics, using configurations based on allmodconfig and
> allyesconfig. WERROR was disabled for both; allyesconfig also had
> KASAN disabled. Both used LD_ORPHAN_WARN_LEVEL="warn" and
> DRM_WERROR=n.
> 
>  net/ipv6/ip6_output.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 25fe0ba98b1a6..843bb95e635b9 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1125,6 +1125,7 @@ static struct dst_entry *ip6_sk_dst_check(struct sock *sk,
>  	    ip6_rt_check(&rt->rt6i_src, &fl6->saddr,
>  			 np->saddr_cache ? &np->saddr : NULL) ||
>  #endif
> +	    fl6->flowi6_mark != READ_ONCE(sk->sk_mark) ||

Asides, not expectation that you fix the following:

The same is probably also true for flowlabel and dscp.

Not net material, but ideally ipv4 and ipv6 would do the exact same
thing.

>  	   (fl6->flowi6_oif && fl6->flowi6_oif != dst_dev(dst)->ifindex)) {
>  		dst_release(dst);
>  		dst = NULL;
> 
> base-commit: d8a4f4c59da52b7c31ae337243b8473a19a3f819
> -- 
> 2.39.0.windows.2
> 
> 
> 

Re: [PATCH net] ipv6: fix cached route reuse for SO_MARK control messages

[Xia Yan]

Hi Willem,

Thanks for the review. I reproduced both on a 6.6-based kernel.

With a connected UDP socket and a cached route, an IPV6_TCLASS cmsg
bypasses a DSCP rule pointing to a prohibit route. Clearing the cache
makes the same send fail with EACCES. The IPv4 case returns EACCES
with or without a cached route.

Under ECMP, changing the per-message flow label keeps using the cached
nexthop. Clearing the cache selects the other nexthop for that label.
Packet captures confirmed the requested traffic class and flow label.

I'll follow up separately for net-next.

Best regards,
Xia