net/mptcp/mptcp_diag.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
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;
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] In net/mptcp/mptcp_diag.c, mptcp_diag_get_info() now only
applies the listener override when the socket is not owned: `if…
--
mptcp: protect listener backlog reads in diagnostics
This commit takes the MPTCP data lock in mptcp_diag_get_info() around the
listener state check and the backlog reads. The override from msk->first
is now applied only when no process owns the socket. The goal is to fix a
use-after-free on the token based diag paths, which can race with a
disconnect, re-listen and close of the same socket.
> diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
> index 136c2d05c0eeb..a71bcaee15c8d 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) {
[Severity: Medium]
mptcp_diag_get_info() is the shared idiag_get_info callback, so this new
sock_owned_by_user() check also applies to mptcp_diag_dump_listeners().
mptcp_listen() never inserts the msk into the token table. Isn't the
listener walk then the main way a listening MPTCP socket gets reported?
While a task holds lock_sock() on the listening msk, the override is
skipped. That covers listen(), most mptcp_setsockopt() and
mptcp_getsockopt() calls, shutdown and close, and the worker. In that
case only the fallback values are reported:
r->idiag_rqueue = sk_rmem_alloc_get(sk) +
READ_ONCE(mptcp_sk(sk)->backlog_len);
r->idiag_wqueue = sk_wmem_alloc_get(sk);
For a listener these are normally 0/0, not the accept queue length and
the max backlog. Could this make ss -Ml and accept queue monitoring show
occasional 0/0 readings for MPTCP listeners?
For comparison, tcp_diag_get_info() always reports sk_ack_backlog and
sk_max_ack_backlog for TCP_LISTEN, with no ownership check:
net/ipv4/tcp_diag.c:tcp_diag_get_info() {
if (inet_sk_state_load(sk) == TCP_LISTEN) {
r->idiag_rqueue = READ_ONCE(sk->sk_ack_backlog);
r->idiag_wqueue = READ_ONCE(sk->sk_max_ack_backlog);
...
}
The listener walk also looks like it was already memory safe before this
patch:
net/mptcp/mptcp_diag.c:mptcp_diag_dump_listeners() {
...
rcu_read_lock();
spin_lock(&ilb->lock);
sk_nulls_for_each(sk, node, &ilb->nulls_head) {
...
ret = sk_diag_dump(sk, skb, cb, r, net_admin);
...
}
Releasing the hashed listener subflow goes through
tcp_set_state(TCP_CLOSE)->inet_unhash(), which takes the same ilb->lock.
Hashed listeners are also SOCK_RCU_FREE.
The use-after-free described in the commit message seems reachable only
from the token based paths: the token iteration in mptcp_diag_dump() and
mptcp_diag_dump_one(). On those paths TCP_LISTEN shows up only in the
disconnect and re-listen race.
The commit message accepts the tradeoff:
While a process owns the socket, the dump reports the MPTCP socket's
own queue values instead.
It doesn't explain why the listener walk needs the check, though. Is the
ownership check needed on the mptcp_diag_dump_listeners() path, or could
it be limited to the token based callers?
With this change, the existing comment inside the if (lsk) block, "so
Send-Q will show accept queue", only holds while the socket is not owned.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/pm-mptcp-objects-candidate-0066-v3-73df72b7a543d4cada0e%40gmail.com
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
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
October 26, 2026 at 10:44 PM, "Cen Zhang" <zzzccc427@gmail.com mailto:zzzccc427@gmail.com?to=%22Cen%20Zhang%22%20%3Czzzccc427%40gmail.com%3E > wrote: > > 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. Hi Cen, Basically this looks fine to me, I don't see any issue with the overall approach. Also, could you send v3 separately to the MPTCP mailing list? It seems the CI checks were not triggered this time. I guess it might be because the patch was not based on the export branch of the mptcp repo on GitHub, so you can generate the patch based on that branch (multipath-tcp/mptcp_net-next, branch export). Two notes for next time (Important): 1. For MPTCP-only patches, it is enough to send them to the MPTCP list only, no need to CC netdev. 2. If a new version is needed, please wait 24 hours before sending it. Thanks Gang > > Best regards, > Cen Zhang >
Hi Gang, Cen, On 27/10/2026 11:41, gang.yan@linux.dev wrote: > October 26, 2026 at 10:44 PM, "Cen Zhang" <zzzccc427@gmail.com mailto:zzzccc427@gmail.com?to=%22Cen%20Zhang%22%20%3Czzzccc427%40gmail.com%3E > wrote: > > >> >> 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. > > Hi Cen, > > Basically this looks fine to me, I don't see any issue with the overall > approach. @Gang: Thank you for your review! (Don't hesitate to send a Reviewed-by/Acked-by on the v2 then ;) ) > Also, could you send v3 separately to the MPTCP mailing list? It seems the > CI checks were not triggered this time. I guess it might be because the > patch was not based on the export branch of the mptcp repo on GitHub, so > you can generate the patch based on that branch > (multipath-tcp/mptcp_net-next, branch export). It's not clear to me why the CI got an issue to apply the v2: Applying: mptcp: protect listener backlog reads in diagnostics error: corrupt patch at line 38 error: could not build fake ancestor (Not sure what this "line 38" corresponds to) https://patchew.org/MPTCP/pm-mptcp-objects-candidate-0066-v5-4d1b83d62fcde2d57e8d@gmail.com/logs/git/ But I manually applied the patch and asked the CI to validate it. @Cen: just to be clear: don't send a v3 just to fix the CI issue, except if a reviewer asks you too. > Two notes for next time (Important): > 1. For MPTCP-only patches, it is enough to send them to the MPTCP list > only, no need to CC netdev. Small note: for "urgent" patches, it is fine to send them to Netdev as well, following what's in the MAINTAINERS file. For new features and non-urgent fixes, it is better to send them to the MPTCP list only to reduce the traffic on the Netdev list. > 2. If a new version is needed, please wait 24 hours before sending it. (or more: wait for the Netdev Sashiko review) Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Matt, Gang, Thanks for the review and guidence. Matthieu Baerts <matttbe@kernel.org> 于2026年10月27日周二 19:37写道: > > > Also, could you send v3 separately to the MPTCP mailing list? It seems the > > CI checks were not triggered this time. I guess it might be because the > > patch was not based on the export branch of the mptcp repo on GitHub, so > > you can generate the patch based on that branch > > (multipath-tcp/mptcp_net-next, branch export). > > @Cen: just to be clear: don't send a v3 just to fix the CI issue, except > if a reviewer asks you too. > Understood. Thanks for applying v2 manually and the guidence. > > Two notes for next time (Important): > > 1. For MPTCP-only patches, it is enough to send them to the MPTCP list > > only, no need to CC netdev. > > Small note: for "urgent" patches, it is fine to send them to Netdev as > well, following what's in the MAINTAINERS file. For new features and > non-urgent fixes, it is better to send them to the MPTCP list only to > reduce the traffic on the Netdev list. > > > 2. If a new version is needed, please wait 24 hours before sending it. > > (or more: wait for the Netdev Sashiko review) Thanks for the clarification. I will follow it in the future. Best regards, Cen Zhang
October 27, 2026 at 6:41 PM, gang.yan@linux.dev mailto:gang.yan@linux.dev wrote: > > October 26, 2026 at 10:44 PM, "Cen Zhang" <zzzccc427@gmail.com mailto:zzzccc427@gmail.com?to=%22Cen%20Zhang%22%20%3Czzzccc427%40gmail.com%3E > wrote: > > > > > 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. > > > Hi Cen, > > Basically this looks fine to me, I don't see any issue with the overall > approach. > Sorry, I replied on the wrong thread, my previous email was actually meant for the v2 patch. Thank Gang > Also, could you send v3 separately to the MPTCP mailing list? It seems the > CI checks were not triggered this time. I guess it might be because the > patch was not based on the export branch of the mptcp repo on GitHub, so > you can generate the patch based on that branch > (multipath-tcp/mptcp_net-next, branch export). > > Two notes for next time (Important): > 1. For MPTCP-only patches, it is enough to send them to the MPTCP list > only, no need to CC netdev. > 2. If a new version is needed, please wait 24 hours before sending it. > > Thanks > Gang > > > > > Best regards, > > Cen Zhang > > >
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.
© 2016 - 2026 Red Hat, Inc.