1. Patchew
  2. MPTCP
  3. View series

[PATCH mptcp-net] mptcp: pm: only set fullmesh for subflow endp

Matthieu Baerts (NGI0) posted 1 patch 4 days, 19 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20250116-mptcp-issue-540-v1-1-b0ba2c126eb8@kernel.org
There is a newer version of this series
net/mptcp/pm_netlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH mptcp-net] mptcp: pm: only set fullmesh for subflow endp
Posted by Matthieu Baerts (NGI0) 4 days, 19 hours ago 
With the in-kernel path-manager, it is possible to change the 'fullmesh'
flag. The code in mptcp_pm_nl_fullmesh() expects to change it only on
'subflow' endpoints, to recreate more or less subflows using the linked
address.

Unfortunately, the set_flags() hook was a bit more permissive, and
allowed 'implicit' endpoints to get the 'fullmesh' flag while it is not
allowed before.

That's what syzbot found, triggering the following warning:

  WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 __mark_subflow_endp_available net/mptcp/pm_netlink.c:1496 [inline]
  WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_pm_nl_fullmesh net/mptcp/pm_netlink.c:1980 [inline]
  WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_nl_set_flags net/mptcp/pm_netlink.c:2003 [inline]
  WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_pm_nl_set_flags+0x974/0xdc0 net/mptcp/pm_netlink.c:2064
  Modules linked in:
  CPU: 0 UID: 0 PID: 6499 Comm: syz.1.413 Not tainted 6.13.0-rc5-syzkaller-00172-gd1bf27c4e176 #0
  Hardware name: Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
  RIP: 0010:__mark_subflow_endp_available net/mptcp/pm_netlink.c:1496 [inline]
  RIP: 0010:mptcp_pm_nl_fullmesh net/mptcp/pm_netlink.c:1980 [inline]
  RIP: 0010:mptcp_nl_set_flags net/mptcp/pm_netlink.c:2003 [inline]
  RIP: 0010:mptcp_pm_nl_set_flags+0x974/0xdc0 net/mptcp/pm_netlink.c:2064
  Code: 01 00 00 49 89 c5 e8 fb 45 e8 f5 e9 b8 fc ff ff e8 f1 45 e8 f5 4c 89 f7 be 03 00 00 00 e8 44 1d 0b f9 eb a0 e8 dd 45 e8 f5 90 <0f> 0b 90 e9 17 ff ff ff 89 d9 80 e1 07 38 c1 0f 8c c9 fc ff ff 48
  RSP: 0018:ffffc9000d307240 EFLAGS: 00010293
  RAX: ffffffff8bb72e03 RBX: 0000000000000000 RCX: ffff88807da88000
  RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
  RBP: ffffc9000d307430 R08: ffffffff8bb72cf0 R09: 1ffff1100b842a5e
  R10: dffffc0000000000 R11: ffffed100b842a5f R12: ffff88801e2e5ac0
  R13: ffff88805c214800 R14: ffff88805c2152e8 R15: 1ffff1100b842a5d
  FS:  00005555619f6500(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 0000000020002840 CR3: 00000000247e6000 CR4: 00000000003526f0
  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
  Call Trace:
   <TASK>
   genl_family_rcv_msg_doit net/netlink/genetlink.c:1115 [inline]
   genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
   genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1210
   netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2542
   genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
   netlink_unicast_kernel net/netlink/af_netlink.c:1321 [inline]
   netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1347
   netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1891
   sock_sendmsg_nosec net/socket.c:711 [inline]
   __sock_sendmsg+0x221/0x270 net/socket.c:726
   ____sys_sendmsg+0x52a/0x7e0 net/socket.c:2583
   ___sys_sendmsg net/socket.c:2637 [inline]
   __sys_sendmsg+0x269/0x350 net/socket.c:2669
   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
   do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
  RIP: 0033:0x7f5fe8785d29
  Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
  RSP: 002b:00007fff571f5558 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
  RAX: ffffffffffffffda RBX: 00007f5fe8975fa0 RCX: 00007f5fe8785d29
  RDX: 0000000000000000 RSI: 0000000020000480 RDI: 0000000000000007
  RBP: 00007f5fe8801b08 R08: 0000000000000000 R09: 0000000000000000
  R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
  R13: 00007f5fe8975fa0 R14: 00007f5fe8975fa0 R15: 00000000000011f4
   </TASK>

Here, syzbot managed to set the 'fullmesh' flag on an 'implicit' and
used -- according to 'id_avail_bitmap' -- endpoint, causing the PM to
try decrement the local_addr_used counter which is only incremented for
the 'subflow' endpoint.

Note that 'no type' endpoints -- not 'subflow', 'signal', 'implicit' --
are fine, because their ID will not be marked as used in the 'id_avail'
bitmap, and setting 'fullmesh' can help forcing the creation of subflow
when receiving an ADD_ADDR.

Fixes: 73c762c1f07d ("mptcp: set fullmesh flag in pm_netlink")
Reported-by: syzbot+cd16e79c1e45f3fe0377@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/6786ac51.050a0220.216c54.00a6.GAE@google.com
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/540
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_netlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index fef01692eaed404e272359df691264f797240d10..7131dc2f312d923a31bb4b0460b6b4663376fbd0 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1986,7 +1986,8 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
 		return -EINVAL;
 	}
 	if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
-	    (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+	    ((entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) ||
+	     (entry->flags & MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
 		spin_unlock_bh(&pernet->lock);
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
 		return -EINVAL;

---
base-commit: 9f10cc97addfb3a81b4aaf4b13f59b36b5bcc7a6
change-id: 20250116-mptcp-issue-540-49d1db3346a9

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>
Re: [PATCH mptcp-net] mptcp: pm: only set fullmesh for subflow endp
Posted by MPTCP CI 4 days, 18 hours ago 
Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: 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/12815258286

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


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 mptcp-net] mptcp: pm: only set fullmesh for subflow endp
Posted by Paolo Abeni 4 days, 19 hours ago 
On 1/16/25 7:15 PM, Matthieu Baerts (NGI0) wrote:
> With the in-kernel path-manager, it is possible to change the 'fullmesh'
> flag. The code in mptcp_pm_nl_fullmesh() expects to change it only on
> 'subflow' endpoints, to recreate more or less subflows using the linked
> address.
> 
> Unfortunately, the set_flags() hook was a bit more permissive, and
> allowed 'implicit' endpoints to get the 'fullmesh' flag while it is not
> allowed before.
> 
> That's what syzbot found, triggering the following warning:
> 
>   WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 __mark_subflow_endp_available net/mptcp/pm_netlink.c:1496 [inline]
>   WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_pm_nl_fullmesh net/mptcp/pm_netlink.c:1980 [inline]
>   WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_nl_set_flags net/mptcp/pm_netlink.c:2003 [inline]
>   WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_pm_nl_set_flags+0x974/0xdc0 net/mptcp/pm_netlink.c:2064
>   Modules linked in:
>   CPU: 0 UID: 0 PID: 6499 Comm: syz.1.413 Not tainted 6.13.0-rc5-syzkaller-00172-gd1bf27c4e176 #0
>   Hardware name: Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
>   RIP: 0010:__mark_subflow_endp_available net/mptcp/pm_netlink.c:1496 [inline]
>   RIP: 0010:mptcp_pm_nl_fullmesh net/mptcp/pm_netlink.c:1980 [inline]
>   RIP: 0010:mptcp_nl_set_flags net/mptcp/pm_netlink.c:2003 [inline]
>   RIP: 0010:mptcp_pm_nl_set_flags+0x974/0xdc0 net/mptcp/pm_netlink.c:2064
>   Code: 01 00 00 49 89 c5 e8 fb 45 e8 f5 e9 b8 fc ff ff e8 f1 45 e8 f5 4c 89 f7 be 03 00 00 00 e8 44 1d 0b f9 eb a0 e8 dd 45 e8 f5 90 <0f> 0b 90 e9 17 ff ff ff 89 d9 80 e1 07 38 c1 0f 8c c9 fc ff ff 48
>   RSP: 0018:ffffc9000d307240 EFLAGS: 00010293
>   RAX: ffffffff8bb72e03 RBX: 0000000000000000 RCX: ffff88807da88000
>   RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
>   RBP: ffffc9000d307430 R08: ffffffff8bb72cf0 R09: 1ffff1100b842a5e
>   R10: dffffc0000000000 R11: ffffed100b842a5f R12: ffff88801e2e5ac0
>   R13: ffff88805c214800 R14: ffff88805c2152e8 R15: 1ffff1100b842a5d
>   FS:  00005555619f6500(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>   CR2: 0000000020002840 CR3: 00000000247e6000 CR4: 00000000003526f0
>   DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>   DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>   Call Trace:
>    <TASK>
>    genl_family_rcv_msg_doit net/netlink/genetlink.c:1115 [inline]
>    genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
>    genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1210
>    netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2542
>    genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
>    netlink_unicast_kernel net/netlink/af_netlink.c:1321 [inline]
>    netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1347
>    netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1891
>    sock_sendmsg_nosec net/socket.c:711 [inline]
>    __sock_sendmsg+0x221/0x270 net/socket.c:726
>    ____sys_sendmsg+0x52a/0x7e0 net/socket.c:2583
>    ___sys_sendmsg net/socket.c:2637 [inline]
>    __sys_sendmsg+0x269/0x350 net/socket.c:2669
>    do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>    do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>   RIP: 0033:0x7f5fe8785d29
>   Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
>   RSP: 002b:00007fff571f5558 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>   RAX: ffffffffffffffda RBX: 00007f5fe8975fa0 RCX: 00007f5fe8785d29
>   RDX: 0000000000000000 RSI: 0000000020000480 RDI: 0000000000000007
>   RBP: 00007f5fe8801b08 R08: 0000000000000000 R09: 0000000000000000
>   R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
>   R13: 00007f5fe8975fa0 R14: 00007f5fe8975fa0 R15: 00000000000011f4
>    </TASK>
> 
> Here, syzbot managed to set the 'fullmesh' flag on an 'implicit' and
> used -- according to 'id_avail_bitmap' -- endpoint, causing the PM to
> try decrement the local_addr_used counter which is only incremented for
> the 'subflow' endpoint.
> 
> Note that 'no type' endpoints -- not 'subflow', 'signal', 'implicit' --
> are fine, because their ID will not be marked as used in the 'id_avail'
> bitmap, and setting 'fullmesh' can help forcing the creation of subflow
> when receiving an ADD_ADDR.
> 
> Fixes: 73c762c1f07d ("mptcp: set fullmesh flag in pm_netlink")
> Reported-by: syzbot+cd16e79c1e45f3fe0377@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/6786ac51.050a0220.216c54.00a6.GAE@google.com
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/540
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_netlink.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index fef01692eaed404e272359df691264f797240d10..7131dc2f312d923a31bb4b0460b6b4663376fbd0 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1986,7 +1986,8 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
>  		return -EINVAL;
>  	}
>  	if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
> -	    (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> +	    ((entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) ||
> +	     (entry->flags & MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
>  		spin_unlock_bh(&pernet->lock);
>  		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
>  		return -EINVAL;

LGTM, thanks!

slight preference for:

		entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL ||
			        MPTCP_PM_ADDR_FLAG_IMPLICIT)

but definitely not strong opinion about it.

/P
Re: [PATCH mptcp-net] mptcp: pm: only set fullmesh for subflow endp
Posted by Matthieu Baerts 4 days, 18 hours ago 
Hi Paolo,

On 16/01/2025 19:37, Paolo Abeni wrote:
> On 1/16/25 7:15 PM, Matthieu Baerts (NGI0) wrote:
>> With the in-kernel path-manager, it is possible to change the 'fullmesh'
>> flag. The code in mptcp_pm_nl_fullmesh() expects to change it only on
>> 'subflow' endpoints, to recreate more or less subflows using the linked
>> address.
>>
>> Unfortunately, the set_flags() hook was a bit more permissive, and
>> allowed 'implicit' endpoints to get the 'fullmesh' flag while it is not
>> allowed before.
>>
>> That's what syzbot found, triggering the following warning:

(...)

>> Fixes: 73c762c1f07d ("mptcp: set fullmesh flag in pm_netlink")
>> Reported-by: syzbot+cd16e79c1e45f3fe0377@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/6786ac51.050a0220.216c54.00a6.GAE@google.com
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/540
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/pm_netlink.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index fef01692eaed404e272359df691264f797240d10..7131dc2f312d923a31bb4b0460b6b4663376fbd0 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -1986,7 +1986,8 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
>>  		return -EINVAL;
>>  	}
>>  	if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
>> -	    (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>> +	    ((entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) ||
>> +	     (entry->flags & MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
>>  		spin_unlock_bh(&pernet->lock);
>>  		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
>>  		return -EINVAL;
> 
> LGTM, thanks!
> 
> slight preference for:
> 
> 		entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL ||
> 			        MPTCP_PM_ADDR_FLAG_IMPLICIT)
> 
> but definitely not strong opinion about it.

Good point! I agree with you. That was in fact my v2, then I switched to
a version with:

  flags & (SUBFLOW | SIGNAL | IMPLICIT) == SUBFLOW

but then I realised the case with no "type" (so not subflow, signal nor
implicit) was OK in fact, even if it will recreate the subflows for no
reasons, but that was already the case before.

So I switched back to v1, and forgot about v2 :)

Can I add your Acked-by or you prefer not to?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.