[PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

[PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

[Kalpan Jani]

syzkaller hit the WARN_ON_ONCE() in mptcp_pm_alloc_anno_list() with
the in-kernel path manager.

When a signal endpoint is removed, the pending ADD_ADDR has to be
cancelled: its retransmit timer stopped and the anno_list entry
unlinked and freed. For a non-zero id endpoint this is done via
mptcp_nl_remove_subflow_and_signal_addr() ->
mptcp_pm_remove_anno_addr() -> mptcp_remove_anno_list_by_saddr().

The id 0 removal path, mptcp_nl_remove_id_zero_address(), does not do
this: it only queues a RM_ADDR and marks the id available again, but
leaves any pending anno_list entry and its armed retransmit timer
alive.

So when the id 0 endpoint is removed and re-added while its previously
sent ADD_ADDR is still awaiting the echo, the stale entry survives.
The kernel PM reselects id 0, reaches mptcp_pm_alloc_anno_list() a
second time, finds the stale entry and hits the WARN.

Make the id 0 removal path symmetric with the non-zero one: drop the
pending ADD_ADDR before queuing the RM_ADDR, and decrement
add_addr_signaled if the address had been announced. This closes the
race at its source, so the WARN_ON_ONCE() stays a valid assertion.

Fixes: 740d798e8767 ("mptcp: remove id 0 address")
Reported-by: syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
---
v1 -> v2:
 - Do not drop the WARN_ON_ONCE(); it is a valid assertion.
   (Matthieu Baerts)
 - Fix mptcp_nl_remove_id_zero_address() to tear down the pending
   ADD_ADDR entry: call mptcp_remove_anno_list_by_saddr() and
   decrement add_addr_signaled, mirroring the non-zero id path.
   (Matthieu Baerts)
v2 -> v3:
 - Shorten the inline comment to one line; full context is in
   the commit message. (Matthieu Baerts)

 net/mptcp/pm_kernel.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index fc818b63752e..128a2e71c26e 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1126,6 +1126,7 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
 		struct sock *sk = (struct sock *)msk;
 		struct mptcp_addr_info msk_local;
+		bool announced;
 
 		if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
 			goto next;
@@ -1135,7 +1136,11 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 			goto next;
 
 		lock_sock(sk);
+		/* Drop a possibly pending ADD_ADDR for this address. */
+		announced = mptcp_remove_anno_list_by_saddr(msk, &msk_local);
 		spin_lock_bh(&msk->pm.lock);
+		if (announced)
+			msk->pm.add_addr_signaled--;
 		mptcp_pm_remove_addr(msk, &list);
 		mptcp_pm_rm_subflow(msk, &list);
 		__mark_subflow_endp_available(msk, 0);
-- 
2.43.0

Re: [PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

[MPTCP CI]

Hi Kalpan,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Unstable: 1 failed test(s): selftest_mptcp_connect_checksum ⚠️ 
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/26628789956

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/6e2e266d3b4b
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1102741


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Hi Kalpan,

First of all, thanks for this patch!

While playing with it on top of the current tree, I think I may have
bumped into a small detail, and I just wanted to mention it in case it
is useful — please do correct me if I got any of this wrong.

After the "mptcp: pm: uniform announced addresses helpers" refactor
(7d4dacc8ccca), the helper became mptcp_pm_announced_remove(), and its
lookup uses mptcp_addresses_equal(..., /* use_port = */ true). A pending
ADD_ADDR entry coming from a signal endpoint added without a port
(`ip mptcp endpoint add ADDR signal`) is keyed with port 1, while
msk_local carries the connection's local port. With a listener on port
5000, a temporary printk in the id 0 path shows:

    msk_local.port = 5000
    announced(port0_lookup) = 1

So, at least in this configuration, passing msk_local directly does not
match the entry: the stale entry survives, and the WARN still shows up
on the next reselection. It only goes away when the port is ignored
during the lookup.

If that matches what you see, the smallest change that keeps the v3
shape would be to clear the port before the lookup, something like:

        struct mptcp_addr_info anno_addr = msk_local;

        anno_addr.port = 0;
        announced = mptcp_pm_announced_remove(msk, &anno_addr);

(An address-only variant of the lookup might be a touch cleaner, since a
signal entry never carries the connection port — but the one-liner above
is enough to fix the case I reproduced.)

I have a small repro that triggers it deterministically if it would help
— happy to share.

Thanks again for looking into this, and sorry for the noise if I'm
missing something obvious here.

Cheers,
Tao

Re: [PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

[Kalpan Jani]

Hi Tao,

Thanks a lot for digging into this -- this is a real bug in the patch, not noise.

You're right: msk_local carries the connection's local port, and the lookup in
mptcp_pm_announced_remove() compares with use_port = true, so passing msk_local
directly fails to match the pending entry from a no-port signal endpoint. The
teardown then never runs and the WARN still fires on reselection. So v3 as-is is
effectively a no-op in this configuration -- thanks for catching it.

I haven't reproduced it deterministically on my side yet, so I'd gladly take you
up on the repro you mentioned -- please share it. I'll run it with a printk in
the id 0 path to confirm the stored entry port and the announced result before
and after the fix.

One detail I want to nail down with that: your note says the entry is "keyed with
port 1", but the suggested fix zeroes the port (anno_addr.port = 0). If the entry
port is really 0, clearing the port as you suggested matches and is the minimal
fix; if it turns out non-zero, zeroing wouldn't match either and the address-only
lookup variant would be needed instead. The repro will let me check which it is.

I'll fold the fix into v4 (rebased onto current export, where the helper is
mptcp_pm_announced_remove()), add a regression test based on the repro, and credit
you with a Suggested-by. If the repro ends up forming the test, I'll coordinate the
right tags with you.

Cheers,
Kalpan Jani


From: Tao Cui <cui.tao@linux.dev>
To: <kalpan.jani@mpiricsoftware.com>
Cc: <janak@mpiric.us>, <kalpanjani009@gmail.com>, <martineau@kernel.org>, <matttbe@kernel.org>, <mptcp@lists.linux.dev>, <pabeni@redhat.com>, <shardul.b@mpiricsoftware.com>, <shardulsb08@gmail.com>, <cui.tao@linux.dev>, <cuitao@kylinos.cn>, <syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com>
Date: Thu, 18 Jun 2026 05:40:34 +0530
Subject: Re: [PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

 > From: Tao Cui <cuitao@kylinos.cn>
 > 
 > Hi Kalpan,
 > 
 > First of all, thanks for this patch!
 > 
 > While playing with it on top of the current tree, I think I may have
 > bumped into a small detail, and I just wanted to mention it in case it
 > is useful — please do correct me if I got any of this wrong.
 > 
 > After the "mptcp: pm: uniform announced addresses helpers" refactor
 > (7d4dacc8ccca), the helper became mptcp_pm_announced_remove(), and its
 > lookup uses mptcp_addresses_equal(..., /* use_port = */ true). A pending
 > ADD_ADDR entry coming from a signal endpoint added without a port
 > (`ip mptcp endpoint add ADDR signal`) is keyed with port 1, while
 > msk_local carries the connection's local port. With a listener on port
 > 5000, a temporary printk in the id 0 path shows:
 > 
 >     msk_local.port = 5000
 >     announced(port0_lookup) = 1
 > 
 > So, at least in this configuration, passing msk_local directly does not
 > match the entry: the stale entry survives, and the WARN still shows up
 > on the next reselection. It only goes away when the port is ignored
 > during the lookup.
 > 
 > If that matches what you see, the smallest change that keeps the v3
 > shape would be to clear the port before the lookup, something like:
 > 
 >         struct mptcp_addr_info anno_addr = msk_local;
 > 
 >         anno_addr.port = 0;
 >         announced = mptcp_pm_announced_remove(msk, &anno_addr);
 > 
 > (An address-only variant of the lookup might be a touch cleaner, since a
 > signal entry never carries the connection port — but the one-liner above
 > is enough to fix the case I reproduced.)
 > 
 > I have a small repro that triggers it deterministically if it would help
 > — happy to share.
 > 
 > Thanks again for looking into this, and sorry for the noise if I'm
 > missing something obvious here.
 > 
 > Cheers,
 > Tao
 > 

Re: [PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint

[Tao Cui]

Hi Kalpan,

Thanks a lot for the kind words, and for the fast and thoughtful reply!

On the entry port: sorry, my printk label was a bit cryptic and probably
misleading -- the stored entry port is 0, not 1. The signal endpoint is
added with no port (`ip mptcp endpoint add 10.0.1.1 signal`), so the
anno_list entry is keyed with port 0; the "=1" in my log line was just
the boolean result of the lookup (entry found), not a port value. So your
instinct is right: clearing the port matches, and that is the minimal fix
for the no-port case.

For completeness, the reasoning I relied on: the lookup inside
mptcp_pm_announced_remove() is mptcp_addresses_equal(..., use_port = true),
and a lookup with port 0 returned "found" -- which can only happen if the
entry's own port is 0. With msk_local (port 5000 here) it returns "not
found", the teardown is skipped and the WARN still fires.

That said, I fully agree with your point about the with-port case: if a
signal endpoint ever carries a port, zeroing wouldn't match either, and
the address-only lookup variant would be the robust choice. Either is fine
by me; I'll defer to whatever you and Matthieu prefer for v4.

Here is the repro, written as a kselftest so it can double as the
regression test you mentioned. I've run it in a QEMU/KVM VM on this tree:
it fails ("not ok", new WARNING: net/mptcp/pm in dmesg) on the unpatched
kernel and passes ("ok") once the id 0 removal drops the pending ADD_ADDR.
The sequence is deterministic:

  - keep the connection alive with a bidirectional /dev/zero stream so
    both ends are fully established;
  - signal 10.0.2.1 so the peer joins -- this gives a second subflow
    (without it the connection is torn down when id 0 is removed) and
    lets mptcp_mpc_endpoint_setup() account the MPC endpoint first, so
    that 10.0.1.1 actually gets selected and announced as id 0;
  - signal 10.0.1.1 (the MPC address) -> anno_list entry created;
  - del id 0 10.0.1.1;
  - signal 10.0.3.1 to force a reselection, which hits the stale entry
    on unpatched kernels.

--->8 (mptcp_id0_stale_anno.sh) ---
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

# Regression test for the syzbot "WARNING in mptcp_pm_alloc_anno_list" bug
# (extid 55c2a5c871441261ed14): removing the id 0 endpoint used to leave a
# pending ADD_ADDR anno_list entry alive, and a later PM reselection
# re-announced that address, tripping the WARN_ON_ONCE(mptcp_pm_is_kernel())
# in mptcp_pm_announced_alloc().
#
# The test keeps an MPTCP connection alive and fully established, then:
#   1. signals 10.0.2.1 so the peer joins (a second subflow keeps the
#      connection alive across the id 0 removal) and the MPC endpoint
#      accounting is done;
#   2. signals the MPC address 10.0.1.1, announced as id 0 (anno_list entry);
#   3. removes id 0 10.0.1.1;
#   4. signals 10.0.3.1 to force a reselection, which hits the stale entry on
#      unpatched kernels.
#
# It fails (new WARNING: net/mptcp/pm in dmesg) on unpatched kernels and passes
# once the id 0 removal drops the pending ADD_ADDR.

. "$(dirname "${0}")/mptcp_lib.sh"

ret=0
ns1=""
ns2=""
err=$(mktemp)
timeout_poll=30
port=50000

# This function is used in the cleanup trap
#shellcheck disable=SC2317,SC2329
cleanup()
{
	rm -f "${err}"
	mptcp_lib_ns_exit "${ns1}" "${ns2}"
}

mptcp_lib_check_mptcp
mptcp_lib_check_tools ip

trap cleanup EXIT

mptcp_lib_ns_init ns1 ns2

# A single veth pair with three subnets: the peer can join each signalled
# address and the connection survives the id 0 removal thanks to the join.
ip link add ns1eth1 netns "${ns1}" type veth peer name ns2eth1 netns "${ns2}"
ip -net "${ns1}" link set lo up
ip -net "${ns2}" link set lo up
ip -net "${ns1}" link set ns1eth1 up
ip -net "${ns2}" link set ns2eth1 up
ip -net "${ns1}" addr add 10.0.1.1/24 dev ns1eth1
ip -net "${ns1}" addr add 10.0.2.1/24 dev ns1eth1
ip -net "${ns1}" addr add 10.0.3.1/24 dev ns1eth1
ip -net "${ns2}" addr add 10.0.1.2/24 dev ns2eth1
ip -net "${ns2}" addr add 10.0.2.2/24 dev ns2eth1
ip -net "${ns2}" addr add 10.0.3.2/24 dev ns2eth1

mptcp_lib_pm_nl_set_limits "${ns1}" 8 8
mptcp_lib_pm_nl_set_limits "${ns2}" 8 8

# Keep the connection alive and fully established on both ends for the whole
# sequence below (a bidirectional /dev/zero stream). It is stopped explicitly
# once the reselection has been forced.
ip netns exec "${ns1}" ./mptcp_connect -t "${timeout_poll}" -l -p "${port}" \
	0.0.0.0 < /dev/zero > /dev/null 2>"${err}" &
spid=$!
mptcp_lib_wait_local_port_listen "${ns1}" "${port}"
ip netns exec "${ns2}" ./mptcp_connect -t "${timeout_poll}" -p "${port}" \
	10.0.1.1 < /dev/zero > /dev/null 2>"${err}" &
cpid=$!

sleep 2

# Count MPTCP PM warnings before the sequence.
warn_before=$(dmesg | grep -c "WARNING: net/mptcp/pm")

# 1. signal 10.0.2.1: peer joins -> second subflow + mpc endpoint accounted.
mptcp_lib_pm_nl_add_endpoint "${ns1}" 10.0.2.1 flags signal
sleep 2
# 2. signal the MPC address 10.0.1.1: announced as id 0 (anno_list entry).
mptcp_lib_pm_nl_add_endpoint "${ns1}" 10.0.1.1 flags signal
sleep 1
# 3. remove id 0: leaves the stale anno_list entry on unpatched kernels.
mptcp_lib_pm_nl_del_endpoint "${ns1}" 0 10.0.1.1
sleep 1
# 4. force a reselection: hits the stale entry on unpatched kernels.
mptcp_lib_pm_nl_add_endpoint "${ns1}" 10.0.3.1 flags signal
sleep 2

warn_after=$(dmesg | grep -c "WARNING: net/mptcp/pm")

# The connection has served its purpose; stop it so the test can finish.
kill "${cpid}" "${spid}" 2>/dev/null
wait "${cpid}" 2>/dev/null
wait "${spid}" 2>/dev/null

if [ "${warn_after}" -gt "${warn_before}" ]; then
	mptcp_lib_result_fail "no stale ADD_ADDR warning on id 0 removal"
	ret=1
else
	mptcp_lib_result_pass "no stale ADD_ADDR warning on id 0 removal"
fi

mptcp_lib_result_print_all_tap

exit ${ret}
---8<---

The temporary printk I used in mptcp_nl_remove_id_zero_address() was:

        anno_addr = msk_local;
        pr_info("DBG id0: msk_local.port=%u\n", ntohs(msk_local.port));
        anno_addr.port = 0;
        announced = mptcp_pm_announced_remove(msk, &anno_addr);
        pr_info("DBG id0: announced(port0_lookup)=%d\n", announced);

which printed `msk_local.port=5000` / `announced(port0_lookup)=1`.

If the repro above ends up forming the test, please feel free to reuse/adapt it as you see
fit.

Happy to test v4 here once it's posted.

在 2026/6/19 15:36, Kalpan Jani 写道:
> 
> Hi Tao,
> 
> Thanks a lot for digging into this -- this is a real bug in the patch, not noise.
> 
> You're right: msk_local carries the connection's local port, and the lookup in
> mptcp_pm_announced_remove() compares with use_port = true, so passing msk_local
> directly fails to match the pending entry from a no-port signal endpoint. The
> teardown then never runs and the WARN still fires on reselection. So v3 as-is is
> effectively a no-op in this configuration -- thanks for catching it.
> 
> I haven't reproduced it deterministically on my side yet, so I'd gladly take you
> up on the repro you mentioned -- please share it. I'll run it with a printk in
> the id 0 path to confirm the stored entry port and the announced result before
> and after the fix.
> 
> One detail I want to nail down with that: your note says the entry is "keyed with
> port 1", but the suggested fix zeroes the port (anno_addr.port = 0). If the entry
> port is really 0, clearing the port as you suggested matches and is the minimal
> fix; if it turns out non-zero, zeroing wouldn't match either and the address-only
> lookup variant would be needed instead. The repro will let me check which it is.
> 
> I'll fold the fix into v4 (rebased onto current export, where the helper is
> mptcp_pm_announced_remove()), add a regression test based on the repro, and credit
> you with a Suggested-by. If the repro ends up forming the test, I'll coordinate the
> right tags with you.
> 
> Cheers,
> Kalpan Jani
> 
> 
> From: Tao Cui <cui.tao@linux.dev>
> To: <kalpan.jani@mpiricsoftware.com>
> Cc: <janak@mpiric.us>, <kalpanjani009@gmail.com>, <martineau@kernel.org>, <matttbe@kernel.org>, <mptcp@lists.linux.dev>, <pabeni@redhat.com>, <shardul.b@mpiricsoftware.com>, <shardulsb08@gmail.com>, <cui.tao@linux.dev>, <cuitao@kylinos.cn>, <syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com>
> Date: Thu, 18 Jun 2026 05:40:34 +0530
> Subject: Re: [PATCH net v3] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint
> 
>  > From: Tao Cui <cuitao@kylinos.cn>
>  > 
>  > Hi Kalpan,
>  > 
>  > First of all, thanks for this patch!
>  > 
>  > While playing with it on top of the current tree, I think I may have
>  > bumped into a small detail, and I just wanted to mention it in case it
>  > is useful — please do correct me if I got any of this wrong.
>  > 
>  > After the "mptcp: pm: uniform announced addresses helpers" refactor
>  > (7d4dacc8ccca), the helper became mptcp_pm_announced_remove(), and its
>  > lookup uses mptcp_addresses_equal(..., /* use_port = */ true). A pending
>  > ADD_ADDR entry coming from a signal endpoint added without a port
>  > (`ip mptcp endpoint add ADDR signal`) is keyed with port 1, while
>  > msk_local carries the connection's local port. With a listener on port
>  > 5000, a temporary printk in the id 0 path shows:
>  > 
>  >     msk_local.port = 5000
>  >     announced(port0_lookup) = 1
>  > 
>  > So, at least in this configuration, passing msk_local directly does not
>  > match the entry: the stale entry survives, and the WARN still shows up
>  > on the next reselection. It only goes away when the port is ignored
>  > during the lookup.
>  > 
>  > If that matches what you see, the smallest change that keeps the v3
>  > shape would be to clear the port before the lookup, something like:
>  > 
>  >         struct mptcp_addr_info anno_addr = msk_local;
>  > 
>  >         anno_addr.port = 0;
>  >         announced = mptcp_pm_announced_remove(msk, &anno_addr);
>  > 
>  > (An address-only variant of the lookup might be a touch cleaner, since a
>  > signal entry never carries the connection port — but the one-liner above
>  > is enough to fix the case I reproduced.)
>  > 
>  > I have a small repro that triggers it deterministically if it would help
>  > — happy to share.
>  > 
>  > Thanks again for looking into this, and sorry for the noise if I'm
>  > missing something obvious here.
>  > 
>  > Cheers,
>  > Tao
>  > 
>