Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets
@ 2026-10-12 10:10 Eric Dumazet
  2026-10-12 10:10 ` [PATCH v3 net 1/2] net: use the full socket in sk_mc_loop() Eric Dumazet
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Eric Dumazet @ 2026-10-12 10:10 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Kuniyuki Iwashima, Neal Cardwell, Ido Schimmel,
	Theodor Arsenij Larionov Trichkine, Simon Horman, netdev,
	Eric Dumazet

Patch 1 fixes an out-of-bounds read in sk_mc_loop() when skb->sk is
a request or a timewait socket, by using the full socket.

Patch 2 fixes the route lookup of cookie_v4_check() when the ACK
carries a source route option, found while looking at the above.

Refusing to send TCP packets to broadcast or multicast destinations
is left for net-next.

v3:
- Back to the v1 approach for the sk_mc_loop() fix: checking the
  destination in TCP (v2) does not cover source routes, multicast
  typed routes on unicast prefixes, or the TIME_WAIT reply path.
- Add the TIME_WAIT case and its Fixes tag.
- Add patch 2.
v2: https://lore.kernel.org/netdev/20261008114942.1376889-1-theodorlarionov@gmail.com/
v1: https://lore.kernel.org/netdev/20261008100423.1256884-1-theodorlarionov@gmail.com/

Eric Dumazet (1):
  tcp: use the echoed source route in cookie_v4_check()

Theodor Arsenij Larionov Trichkine (1):
  net: use the full socket in sk_mc_loop()

 net/core/sock.c       |  1 +
 net/ipv4/syncookies.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 net 1/2] net: use the full socket in sk_mc_loop()
  2026-10-12 10:10 [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets Eric Dumazet
@ 2026-10-12 10:10 ` Eric Dumazet
  2026-10-13  1:09   ` Kuniyuki Iwashima
  2026-10-12 10:10 ` [PATCH v3 net 2/2] tcp: use the echoed source route in cookie_v4_check() Eric Dumazet
  2026-10-12 10:14 ` [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets netdev-bot+sinfo
  2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-10-12 10:10 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Kuniyuki Iwashima, Neal Cardwell, Ido Schimmel,
	Theodor Arsenij Larionov Trichkine, Simon Horman, netdev,
	Eric Dumazet

From: Theodor Arsenij Larionov Trichkine <theodorlarionov@gmail.com>

sk_mc_loop() is called from ip_mc_output() and ip6_finish_output2()
with skb->sk, which is not always a full socket:

- Since commit ca6fb0651883 ("tcp: attach SYNACK messages to request
  sockets instead of listener"), a SYNACK is attached to its request
  socket.

- Since commit 79636038d37e ("ipv4: tcp: give socket pointer to control
  skbs"), an ACK or RST sent by ip_send_unicast_reply() on behalf of a
  TIME_WAIT socket is attached to the timewait socket.

When such a packet uses a multicast route, inet_test_bit(MC_LOOP, sk)
reads inet_flags past the end of the request or timewait socket.

This happens for instance when a SYN with a multicast source address
reaches a listener, after being looped back with its dst attached, so
that ip_rcv_finish_core() skips the martian source check. A raw
IP_HDRINCL socket sending to a local address does this, as does
re-injection by nft dup or the iptables TEE target. The destination
can also come from a source route option, or from a route of type
multicast covering a unicast prefix.

BUG: KASAN: slab-out-of-bounds in sk_mc_loop+0x111/0x170
Read of size 8 at addr ffff88800f2b7f10 by task repro-raw/470
Call Trace:
 <IRQ>
 sk_mc_loop+0x111/0x170
 ip_mc_output+0x355/0x930
 ip_build_and_send_pkt+0xb87/0xc50
 tcp_v4_send_synack+0x500/0x6f0
 tcp_conn_request+0x2135/0x2d90
 tcp_v4_conn_request+0xa5/0x210
 tcp_rcv_state_process+0x136e/0x6920
 tcp_v4_do_rcv+0x339/0xb10
 tcp_v4_rcv+0x34ab/0x3ab0
 ip_protocol_deliver_rcu+0x6e/0x3e0
 ip_local_deliver_finish+0x34d/0x510
 ip_local_deliver+0x1bc/0x310
 ip_rcv+0x390/0x410
 __netif_receive_skb_one_core+0x199/0x1e0
 process_backlog+0x239/0x680
 __napi_poll+0xb5/0x650
 net_rx_action+0x980/0xd60
 handle_softirqs+0x17f/0x590
 do_softirq+0x3f/0x60
 </IRQ>

The buggy address belongs to the object at ffff88800f2b7d60
 which belongs to the cache request_sock_TCP of size 312
The buggy address is located 120 bytes to the right of
 allocated 312-byte region [ffff88800f2b7d60, ffff88800f2b7e98)

Use sk_const_to_full_sk(), which restores the behavior from before
the two commits above: a request socket uses the setting of its
listener, and a timewait socket gets the default (loop back), as the
per-cpu control socket used before had.

Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener")
Fixes: 79636038d37e ("ipv4: tcp: give socket pointer to control skbs")
Signed-off-by: Theodor Arsenij Larionov Trichkine <theodorlarionov@gmail.com>
[edumazet: rewrote the changelog, added the TIME_WAIT case]
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
v3:
- Back to the v1 approach: checking the destination in TCP (v2) does
  not cover source routes, multicast typed routes on unicast prefixes,
  or the TIME_WAIT reply path (sashiko).
- Add the TIME_WAIT case and its Fixes tag.
v2: https://lore.kernel.org/netdev/20261008114942.1376889-1-theodorlarionov@gmail.com/
v1: https://lore.kernel.org/netdev/20261008100423.1256884-1-theodorlarionov@gmail.com/

 net/core/sock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/sock.c b/net/core/sock.c
index e8551df8330ff4b1d662b0d0133e20d1f74eb728..1302c1e848b2a3c40e86ff37af9c20c1a9f4662a 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. */
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 net 2/2] tcp: use the echoed source route in cookie_v4_check()
  2026-10-12 10:10 [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets Eric Dumazet
  2026-10-12 10:10 ` [PATCH v3 net 1/2] net: use the full socket in sk_mc_loop() Eric Dumazet
@ 2026-10-12 10:10 ` Eric Dumazet
  2026-10-13  1:25   ` Kuniyuki Iwashima
  2026-10-12 10:14 ` [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets netdev-bot+sinfo
  2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-10-12 10:10 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Kuniyuki Iwashima, Neal Cardwell, Ido Schimmel,
	Theodor Arsenij Larionov Trichkine, Simon Horman, netdev,
	Eric Dumazet

When the ACK carries a source route option, cookie_v4_check() routes
the new socket to opt->faddr, where opt points to the options of the
received packet.

ip_options_compile() only fills faddr for options given by
setsockopt() (when called without an skb), so on receive faddr is 0
and the lookup is done for daddr 0, which gives a local route.

Use the options echoed by tcp_v4_save_options() instead, as
inet_csk_route_req() and inet_csk_route_child_sock() do with
ireq->ireq_opt. Read them after security_inet_conn_request(),
since CIPSO can replace ireq->ireq_opt there.

The route now goes to the first hop of the source route, so also
refuse a strict source route through a gateway, as
inet_csk_route_req() does.

Found by sashiko while reviewing the sk_mc_loop() fix.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
v3: new patch.

 net/ipv4/syncookies.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 73e12976818473277e83ab47f271a43991217e3e..8c9691569e01af0140b8db8990ceeddc3c5fd8fb 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -404,12 +404,12 @@ static struct request_sock *cookie_tcp_check(struct net *net, struct sock *sk,
  */
 struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 {
-	struct ip_options *opt = &TCP_SKB_CB(skb)->header.h4.opt;
 	const struct tcphdr *th = tcp_hdr(skb);
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct inet_request_sock *ireq;
 	struct net *net = sock_net(sk);
 	struct tcp_request_sock *treq;
+	struct ip_options_rcu *opt;
 	struct request_sock *req;
 	struct sock *ret = sk;
 	struct flowi4 fl4;
@@ -449,6 +449,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 		SKB_DR_SET(reason, SECURITY_HOOK);
 		goto out_free;
 	}
+	/* Load ireq->ireq_opt after the LSM hook, which can replace it. */
+	opt = rcu_dereference_protected(ireq->ireq_opt, true);
 
 	tcp_ao_syncookie(sk, skb, req, AF_INET);
 
@@ -461,7 +463,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	flowi4_init_output(&fl4, ireq->ir_iif, ireq->ir_mark,
 			   ip_sock_rt_tos(sk), ip_sock_rt_scope(sk),
 			   IPPROTO_TCP, inet_sk_flowi_flags(sk),
-			   opt->srr ? opt->faddr : ireq->ir_rmt_addr,
+			   (opt && opt->opt.srr) ? opt->opt.faddr : ireq->ir_rmt_addr,
 			   ireq->ir_loc_addr, th->source, th->dest,
 			   sk_uid(sk));
 	security_req_classify_flow(req, flowi4_to_flowi_common(&fl4));
@@ -470,6 +472,12 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 		SKB_DR_SET(reason, IP_OUTNOROUTES);
 		goto out_free;
 	}
+	/* Same check as in inet_csk_route_req(). */
+	if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway) {
+		ip_rt_put(rt);
+		SKB_DR_SET(reason, IP_OUTNOROUTES);
+		goto out_free;
+	}
 
 	/* Try to redo what tcp_v4_send_synack did. */
 	req->rsk_window_clamp = READ_ONCE(tp->window_clamp) ? :
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets
  2026-10-12 10:10 [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets Eric Dumazet
  2026-10-12 10:10 ` [PATCH v3 net 1/2] net: use the full socket in sk_mc_loop() Eric Dumazet
  2026-10-12 10:10 ` [PATCH v3 net 2/2] tcp: use the echoed source route in cookie_v4_check() Eric Dumazet
@ 2026-10-12 10:14 ` netdev-bot+sinfo
  2 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sinfo @ 2026-10-12 10:14 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
	Neal Cardwell, Ido Schimmel, Theodor Arsenij Larionov Trichkine,
	Simon Horman, netdev

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 net 1/2] net: use the full socket in sk_mc_loop()
  2026-10-12 10:10 ` [PATCH v3 net 1/2] net: use the full socket in sk_mc_loop() Eric Dumazet
@ 2026-10-13  1:09   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-13  1:09 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Neal Cardwell,
	Ido Schimmel, Theodor Arsenij Larionov Trichkine, Simon Horman,
	netdev

On Mon, Oct 12, 2026 at 3:10 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> From: Theodor Arsenij Larionov Trichkine <theodorlarionov@gmail.com>
>
> sk_mc_loop() is called from ip_mc_output() and ip6_finish_output2()
> with skb->sk, which is not always a full socket:
>
> - Since commit ca6fb0651883 ("tcp: attach SYNACK messages to request
>   sockets instead of listener"), a SYNACK is attached to its request
>   socket.
>
> - Since commit 79636038d37e ("ipv4: tcp: give socket pointer to control
>   skbs"), an ACK or RST sent by ip_send_unicast_reply() on behalf of a
>   TIME_WAIT socket is attached to the timewait socket.
>
> When such a packet uses a multicast route, inet_test_bit(MC_LOOP, sk)
> reads inet_flags past the end of the request or timewait socket.
>
> This happens for instance when a SYN with a multicast source address
> reaches a listener, after being looped back with its dst attached, so
> that ip_rcv_finish_core() skips the martian source check. A raw
> IP_HDRINCL socket sending to a local address does this, as does
> re-injection by nft dup or the iptables TEE target. The destination
> can also come from a source route option, or from a route of type
> multicast covering a unicast prefix.
>
> BUG: KASAN: slab-out-of-bounds in sk_mc_loop+0x111/0x170
> Read of size 8 at addr ffff88800f2b7f10 by task repro-raw/470
> Call Trace:
>  <IRQ>
>  sk_mc_loop+0x111/0x170
>  ip_mc_output+0x355/0x930
>  ip_build_and_send_pkt+0xb87/0xc50
>  tcp_v4_send_synack+0x500/0x6f0
>  tcp_conn_request+0x2135/0x2d90
>  tcp_v4_conn_request+0xa5/0x210
>  tcp_rcv_state_process+0x136e/0x6920
>  tcp_v4_do_rcv+0x339/0xb10
>  tcp_v4_rcv+0x34ab/0x3ab0
>  ip_protocol_deliver_rcu+0x6e/0x3e0
>  ip_local_deliver_finish+0x34d/0x510
>  ip_local_deliver+0x1bc/0x310
>  ip_rcv+0x390/0x410
>  __netif_receive_skb_one_core+0x199/0x1e0
>  process_backlog+0x239/0x680
>  __napi_poll+0xb5/0x650
>  net_rx_action+0x980/0xd60
>  handle_softirqs+0x17f/0x590
>  do_softirq+0x3f/0x60
>  </IRQ>
>
> The buggy address belongs to the object at ffff88800f2b7d60
>  which belongs to the cache request_sock_TCP of size 312
> The buggy address is located 120 bytes to the right of
>  allocated 312-byte region [ffff88800f2b7d60, ffff88800f2b7e98)
>
> Use sk_const_to_full_sk(), which restores the behavior from before
> the two commits above: a request socket uses the setting of its
> listener, and a timewait socket gets the default (loop back), as the
> per-cpu control socket used before had.
>
> Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener")
> Fixes: 79636038d37e ("ipv4: tcp: give socket pointer to control skbs")
> Signed-off-by: Theodor Arsenij Larionov Trichkine <theodorlarionov@gmail.com>
> [edumazet: rewrote the changelog, added the TIME_WAIT case]
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 net 2/2] tcp: use the echoed source route in cookie_v4_check()
  2026-10-12 10:10 ` [PATCH v3 net 2/2] tcp: use the echoed source route in cookie_v4_check() Eric Dumazet
@ 2026-10-13  1:25   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-13  1:25 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Neal Cardwell,
	Ido Schimmel, Theodor Arsenij Larionov Trichkine, Simon Horman,
	netdev

On Mon, Oct 12, 2026 at 3:10 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> When the ACK carries a source route option, cookie_v4_check() routes
> the new socket to opt->faddr, where opt points to the options of the
> received packet.
>
> ip_options_compile() only fills faddr for options given by
> setsockopt() (when called without an skb), so on receive faddr is 0
> and the lookup is done for daddr 0, which gives a local route.
>
> Use the options echoed by tcp_v4_save_options() instead, as
> inet_csk_route_req() and inet_csk_route_child_sock() do with
> ireq->ireq_opt. Read them after security_inet_conn_request(),
> since CIPSO can replace ireq->ireq_opt there.
>
> The route now goes to the first hop of the source route, so also
> refuse a strict source route through a gateway, as
> inet_csk_route_req() does.
>
> Found by sashiko while reviewing the sk_mc_loop() fix.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-13  1:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 10:10 [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets Eric Dumazet
2026-10-12 10:10 ` [PATCH v3 net 1/2] net: use the full socket in sk_mc_loop() Eric Dumazet
2026-10-13  1:09   ` Kuniyuki Iwashima
2026-10-12 10:10 ` [PATCH v3 net 2/2] tcp: use the echoed source route in cookie_v4_check() Eric Dumazet
2026-10-13  1:25   ` Kuniyuki Iwashima
2026-10-12 10:14 ` [PATCH v3 net 0/2] net: fix sk_mc_loop() with request and timewait sockets netdev-bot+sinfo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox