Re: [PATCH net] net: use the full socket in sk_mc_loop()

[Eric Dumazet]

Le jeu. 8 oct. 2026 à 12:04, Theodor Arsenij Larionov Trichkine
<theodorlarionov@xxxxxxxxx> a écrit :
>
> tcp_make_synack() sets skb->sk of a SYN-ACK to the request socket.
> If the SYN-ACK destination is multicast, ip_mc_output() calls
> sk_mc_loop() on it, and inet_test_bit(MC_LOOP, sk) reads inet_flags,
> which is past the end of the smaller request_sock object.
>
> KASAN reports a slab-out-of-bounds (or slab-use-after-free) read in
> sk_mc_loop() from tcp_v4_send_synack() when a SYN with a multicast
> source address reaches a listener.
>

How does such a SYN reach a listener ?

ip_route_input_slow() rejects a multicast saddr as a martian source,
so this can not come from the wire.

I guess this needs a local sender : raw socket with IP_HDRINCL, and
the packet going through loopback, where the dst is kept
(skb_dst_force() in loopback_xmit()), so that ip_rcv_finish_core()
skips ip_route_input_noref().

Also the SYNACK route only uses ip_mc_output() if RTCF_LOCAL is set
and the output device is not loopback, so the SYN must target an
address of a non loopback device, and the source must be a group
joined on this device.

Please describe this in the changelog, and include the (trimmed)
KASAN splat, and a repro if you have one.

(Was it a public syzbot report ?)

> Map the socket to the full socket before reading its flags.
>
> Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener")
> Signed-off-by: Theodor Arsenij Larionov Trichkine <theodorlarionov@xxxxxxxxx>
> ---
>  net/core/sock.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index e8551df8330f..1302c1e848b2 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -769,6 +769,7 @@ bool sk_mc_loop(const struct sock *sk)
>  {
>         if (dev_recursion_level())
>                 return false;
> +       sk = sk_const_to_full_sk(sk);
>         if (!sk)
>                 return true;
>         /* IPV6_ADDRFORM can change sk->sk_family under us. */
>

This only addresses the out-of-bounds read, masking the root cause.

We still answer a SYN with a multicast source. With this patch the
SYNACK is looped back depending on the listener MC_LOOP setting,
and syncookie SYNACK (skb->sk == NULL) were already sent.

tcp_v4_conn_request() only looks at the input route flags, ie the
destination of the SYN. tcp_v4_connect() refuses RTCF_MULTICAST and
RTCF_BROADCAST routes, I think we want the same check on the route
used for the SYNACK.

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 6a30f11384547e6adaeff13a005c7ce1d316a602..aa928015cd143234d3d0c72e2b607d8347f73269
100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -779,6 +779,9 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
                goto no_route;
        if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway)
                goto route_err;
+       /* Never send a SYNACK to a broadcast or multicast destination. */
+       if (rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
+               goto route_err;
        rcu_read_unlock();
        return &rt->dst;

pw-bot: cr