mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] mptcp: protect listener backlog reads in diagnostics
@ 2026-10-26 12:04 Cen Zhang
  2026-10-26 12:10 ` netdev-bot+sinfo
  2026-10-26 12:38 ` Eric Dumazet
  0 siblings, 2 replies; 4+ messages in thread
From: Cen Zhang @ 2026-10-26 12:04 UTC (permalink / raw)
  To: matttbe, martineau, davem, edumazet, kuba, pabeni
  Cc: geliang, horms, netdev, mptcp, linux-kernel, baijiaju1990,
	jjzuming, zzzccc427

mptcp_diag_get_info() reports the accept queue of a listening MPTCP
socket by reading msk->first. The token based dump and the exact token
query only hold a reference on the MPTCP socket, which pins neither the
first subflow nor the listener hash bucket, and __mptcp_close_ssk()
drops the last reference on the first subflow before clearing
msk->first.

An application can disconnect a connected MPTCP socket with AF_UNSPEC,
which removes the token but retains the first subflow, listen on it
again and close it. A dump that obtained the MPTCP socket before the
disconnect then reads the backlog of a freed TCP listener:

  Token diagnostic                 Application
  mptcp_token_iter_next()
    retain MPTCP socket
                                   connect(AF_UNSPEC)
                                     remove token, retain first
                                   listen()
                                     reuse first
  mptcp_diag_get_info()
    observe TCP_LISTEN
    lsk = msk->first
                                   close()
                                     mptcp_destroy()
                                       __mptcp_close_ssk()
                                         release first
    read lsk->sk_ack_backlog

Take the MPTCP data lock around the listener state check and the
backlog reads, and only follow msk->first while no process owns the
socket. Every path that disposes of the first subflow runs with the
socket owned, and lock_sock() has to take the same spinlock to become
the owner, so the reads cannot overlap with the release. Waiting for
the owner is not an option: mptcp_diag_dump_listeners() calls this
function with the listener hash bucket lock held, which the owner needs
to unhash that very listener. While a process owns the socket, the dump
reports the MPTCP socket's own queue values instead.

BUG: KASAN: slab-use-after-free in mptcp_diag_get_info+0x415/0x490
Read of size 4 at addr ffff888111763818 by task python3/499

Call Trace:
 <TASK>
 mptcp_diag_get_info+0x415/0x490
 inet_sk_diag_fill+0xee3/0x1ec0
 mptcp_diag_dump+0x2eb/0x1140
 __inet_diag_dump+0x18c/0x2d0
 netlink_dump+0x4c6/0x11f0
 __netlink_dump_start+0x69a/0x920
 inet_diag_handler_cmd+0x254/0x2b0
 sock_diag_rcv_msg+0x3c9/0x730
 netlink_rcv_skb+0x133/0x390
 netlink_unicast+0x504/0x840
 netlink_sendmsg+0x7f7/0xcf0
 __sys_sendto+0x425/0x470
 __x64_sys_sendto+0xe5/0x1c0
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
 </TASK>

Allocated by task 497:
 kmem_cache_alloc_noprof+0x20b/0x5f0
 sk_prot_alloc+0x65/0x260
 sk_alloc+0x37/0xb40
 inet_create+0x33a/0xf40
 __sock_create+0x2c4/0x730
 mptcp_subflow_create_socket+0xf1/0x9a0
 __mptcp_nmpc_sk.part.0+0x114/0x750
 __mptcp_nmpc_sk+0x5c/0x80
 mptcp_setsockopt+0x5d8/0x2d60
 do_sock_setsockopt+0x1fb/0x430
 __sys_setsockopt+0x106/0x180
 __x64_sys_setsockopt+0xc2/0x160
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 0:
 slab_free_after_rcu_debug+0x111/0x290
 rcu_core+0x661/0x1d10
 handle_softirqs+0x201/0x930
 __irq_exit_rcu+0x110/0x1e0
 irq_exit_rcu+0xe/0x20
 sysvec_apic_timer_interrupt+0x6c/0x80
 asm_sysvec_apic_timer_interrupt+0x1a/0x20

Second to last potentially related work creation:
 __call_rcu_common.constprop.0+0x76/0xbd0
 sk_destruct+0x8e/0xd0
 __sk_free+0xdd/0x370
 sk_free+0x51/0x80
 __mptcp_close_ssk+0x903/0x1580
 mptcp_destroy_common+0x16b/0x3c0
 mptcp_destroy+0x4b/0xc0
 __mptcp_destroy_sock+0x10e/0x2f0
 __mptcp_close+0x5df/0xa50
 mptcp_close+0x34/0x100
 inet_release+0x11f/0x250
 __sock_release+0xb8/0x270
 sock_close+0x21/0x30
 __fput+0x39f/0xa60
 fput_close_sync+0xff/0x200
 __x64_sys_close+0x8c/0xf0
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fixes: 4fa39b701ce9 ("mptcp: listen diag dump support")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Assisted-by: LLM
---
 net/mptcp/mptcp_diag.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index 136c2d05c0ee..a71bcaee15c8 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -199,7 +199,12 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
 			  READ_ONCE(mptcp_sk(sk)->backlog_len);
 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
 
-	if (inet_sk_state_load(sk) == TCP_LISTEN) {
+	/* The listener walk holds the listen hash lock: do not wait for a
+	 * socket owner that could be unhashing the first subflow.
+	 */
+	mptcp_data_lock(sk);
+	if (!sock_owned_by_user(sk) &&
+	    inet_sk_state_load(sk) == TCP_LISTEN) {
 		struct sock *lsk = READ_ONCE(msk->first);
 
 		if (lsk) {
@@ -210,6 +215,7 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
 			r->idiag_wqueue = READ_ONCE(lsk->sk_max_ack_backlog);
 		}
 	}
+	mptcp_data_unlock(sk);
 
 	if (!info)
 		return;

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

* Re: [PATCH net] mptcp: protect listener backlog reads in diagnostics
  2026-10-26 12:04 [PATCH net] mptcp: protect listener backlog reads in diagnostics Cen Zhang
@ 2026-10-26 12:10 ` netdev-bot+sinfo
  2026-10-26 12:38 ` Eric Dumazet
  1 sibling, 0 replies; 4+ messages in thread
From: netdev-bot+sinfo @ 2026-10-26 12:10 UTC (permalink / raw)
  To: Cen Zhang
  Cc: matttbe, martineau, davem, edumazet, kuba, pabeni, geliang,
	horms, netdev, mptcp, linux-kernel, baijiaju1990, jjzuming

Hi!

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

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

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] 4+ messages in thread

* Re: [PATCH net] mptcp: protect listener backlog reads in diagnostics
  2026-10-26 12:04 [PATCH net] mptcp: protect listener backlog reads in diagnostics Cen Zhang
  2026-10-26 12:10 ` netdev-bot+sinfo
@ 2026-10-26 12:38 ` Eric Dumazet
  2026-10-26 14:44   ` Cen Zhang
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2026-10-26 12:38 UTC (permalink / raw)
  To: Cen Zhang
  Cc: matttbe, martineau, davem, kuba, pabeni, geliang, horms, netdev,
	mptcp, linux-kernel, baijiaju1990, jjzuming

Le lun. 26 oct. 2026 à 13:04, Cen Zhang <zzzccc427@gmail.com> a écrit :
>
> mptcp_diag_get_info() reports the accept queue of a listening MPTCP
> socket by reading msk->first. The token based dump and the exact token
> query only hold a reference on the MPTCP socket, which pins neither the
> first subflow nor the listener hash bucket, and __mptcp_close_ssk()
> drops the last reference on the first subflow before clearing
> msk->first.
>
> An application can disconnect a connected MPTCP socket with AF_UNSPEC,
> which removes the token but retains the first subflow, listen on it
> again and close it. A dump that obtained the MPTCP socket before the
> disconnect then reads the backlog of a freed TCP listener:
>
>   Token diagnostic                 Application
>   mptcp_token_iter_next()
>     retain MPTCP socket
>                                    connect(AF_UNSPEC)
>                                      remove token, retain first
>                                    listen()
>                                      reuse first
>   mptcp_diag_get_info()
>     observe TCP_LISTEN
>     lsk = msk->first
>                                    close()
>                                      mptcp_destroy()
>                                        __mptcp_close_ssk()
>                                          release first
>     read lsk->sk_ack_backlog
>
> Take the MPTCP data lock around the listener state check and the
> backlog reads, and only follow msk->first while no process owns the
> socket. Every path that disposes of the first subflow runs with the
> socket owned, and lock_sock() has to take the same spinlock to become
> the owner, so the reads cannot overlap with the release. Waiting for
> the owner is not an option: mptcp_diag_dump_listeners() calls this
> function with the listener hash bucket lock held, which the owner needs
> to unhash that very listener. While a process owns the socket, the dump
> reports the MPTCP socket's own queue values instead.
>
> BUG: KASAN: slab-use-after-free in mptcp_diag_get_info+0x415/0x490
> Read of size 4 at addr ffff888111763818 by task python3/499
>
> Call Trace:

Adding these stack traces without symbols is not really nice.

Time to use scripts/decode_stacktrace.sh

>
> Fixes: 4fa39b701ce9 ("mptcp: listen diag dump support")
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> Assisted-by: LLM
> ---
>  net/mptcp/mptcp_diag.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
> index 136c2d05c0ee..a71bcaee15c8 100644
> --- a/net/mptcp/mptcp_diag.c
> +++ b/net/mptcp/mptcp_diag.c
> @@ -199,7 +199,12 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
>                           READ_ONCE(mptcp_sk(sk)->backlog_len);
>         r->idiag_wqueue = sk_wmem_alloc_get(sk);
>
> -       if (inet_sk_state_load(sk) == TCP_LISTEN) {
> +       /* The listener walk holds the listen hash lock: do not wait for a
> +        * socket owner that could be unhashing the first subflow.
> +        */
> +       mptcp_data_lock(sk);
> +       if (!sock_owned_by_user(sk) &&
> +           inet_sk_state_load(sk) == TCP_LISTEN) {
>                 struct sock *lsk = READ_ONCE(msk->first);
>
>                 if (lsk) {
> @@ -210,6 +215,7 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
>                         r->idiag_wqueue = READ_ONCE(lsk->sk_max_ack_backlog);
>                 }
>         }
> +       mptcp_data_unlock(sk);
>
>         if (!info)
>                 return;

This seems overkill, and adds a new msk slock -> ilb->lock nesting from
mptcp_diag_dump_listeners(). It also makes ss randomly report msk
rmem/wmem instead of the accept queue while the listener is owned.

msk->first is a TCP socket: TCP listeners have SOCK_RCU_FREE, and
tcp_prot / tcpv6_prot use SLAB_TYPESAFE_BY_RCU. Reading two integers
under rcu_read_lock() is safe, and stale values are fine for diag.

mptcp_diag_dump_listeners() already holds rcu_read_lock(), only the
token dump and mptcp_diag_dump_one() paths are missing it.

Something like:

diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index 136c2d05c0eeb8f7050344a63396396d8718206c..28a42eee665129fe56c47dc2dd5b265ba18b2e95
100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -200,8 +200,10 @@ static void mptcp_diag_get_info(struct sock *sk,
struct inet_diag_msg *r,
        r->idiag_wqueue = sk_wmem_alloc_get(sk);

        if (inet_sk_state_load(sk) == TCP_LISTEN) {
-               struct sock *lsk = READ_ONCE(msk->first);
+               struct sock *lsk;

+               rcu_read_lock();
+               lsk = READ_ONCE(msk->first);
                if (lsk) {
                        /* override with settings from tcp listener,
                         * so Send-Q will show accept queue.
@@ -209,6 +211,7 @@ static void mptcp_diag_get_info(struct sock *sk,
struct inet_diag_msg *r,
                        r->idiag_rqueue = READ_ONCE(lsk->sk_ack_backlog);
                        r->idiag_wqueue = READ_ONCE(lsk->sk_max_ack_backlog);
                }
+               rcu_read_unlock();
        }

        if (!info)


pw-bot: cr

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

* Re: [PATCH net] mptcp: protect listener backlog reads in diagnostics
  2026-10-26 12:38 ` Eric Dumazet
@ 2026-10-26 14:44   ` Cen Zhang
  0 siblings, 0 replies; 4+ messages in thread
From: Cen Zhang @ 2026-10-26 14:44 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: matttbe, martineau, davem, kuba, pabeni, geliang, horms, netdev,
	mptcp, linux-kernel, baijiaju1990, jjzuming

Hi Eric,

Eric Dumazet <edumazet@kernel.org> 于2026年10月26日周一 20:38写道:
>
> Adding these stack traces without symbols is not really nice.
>
> Time to use scripts/decode_stacktrace.sh

Sorry about that, v2 carries the report decoded with
scripts/decode_stacktrace.sh and trimmed to the relevant frames.

> msk->first is a TCP socket: TCP listeners have SOCK_RCU_FREE, and
> tcp_prot / tcpv6_prot use SLAB_TYPESAFE_BY_RCU. Reading two integers
> under rcu_read_lock() is safe, and stale values are fine for diag.

Thanks for the suggestions.

Best regards,
Cen Zhang

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

end of thread, other threads:[~2026-10-26 14:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-26 12:04 [PATCH net] mptcp: protect listener backlog reads in diagnostics Cen Zhang
2026-10-26 12:10 ` netdev-bot+sinfo
2026-10-26 12:38 ` Eric Dumazet
2026-10-26 14:44   ` Cen Zhang

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

all inboxes | Powered by JetHome®