1
With the in-kernel path-manager, it is possible to change the 'fullmesh'
1
Here are a few patches linked to the 'set-flags' interface with the
2
flag. The code in mptcp_pm_nl_fullmesh() expects to change it only on
2
in-kernel path-manager.
3
'subflow' endpoints, to recreate more or less subflows using the linked
4
address.
5
3
6
Unfortunately, the set_flags() hook was a bit more permissive, and
4
Patch 1: a small cleanup. For -next.
7
allowed 'implicit' endpoints to get the 'fullmesh' flag while it is not
8
allowed before.
9
5
10
That's what syzbot found, triggering the following warning:
6
Patch 2: a small optimisation not to iterate over all subflows to do
7
nothing. For -next.
11
8
12
WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 __mark_subflow_endp_available net/mptcp/pm_netlink.c:1496 [inline]
13
WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_pm_nl_fullmesh net/mptcp/pm_netlink.c:1980 [inline]
14
WARNING: CPU: 0 PID: 6499 at net/mptcp/pm_netlink.c:1496 mptcp_nl_set_flags net/mptcp/pm_netlink.c:2003 [inline]
15
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
16
Modules linked in:
17
CPU: 0 UID: 0 PID: 6499 Comm: syz.1.413 Not tainted 6.13.0-rc5-syzkaller-00172-gd1bf27c4e176 #0
18
Hardware name: Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
19
RIP: 0010:__mark_subflow_endp_available net/mptcp/pm_netlink.c:1496 [inline]
20
RIP: 0010:mptcp_pm_nl_fullmesh net/mptcp/pm_netlink.c:1980 [inline]
21
RIP: 0010:mptcp_nl_set_flags net/mptcp/pm_netlink.c:2003 [inline]
22
RIP: 0010:mptcp_pm_nl_set_flags+0x974/0xdc0 net/mptcp/pm_netlink.c:2064
23
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
24
RSP: 0018:ffffc9000d307240 EFLAGS: 00010293
25
RAX: ffffffff8bb72e03 RBX: 0000000000000000 RCX: ffff88807da88000
26
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
27
RBP: ffffc9000d307430 R08: ffffffff8bb72cf0 R09: 1ffff1100b842a5e
28
R10: dffffc0000000000 R11: ffffed100b842a5f R12: ffff88801e2e5ac0
29
R13: ffff88805c214800 R14: ffff88805c2152e8 R15: 1ffff1100b842a5d
30
FS: 00005555619f6500(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
31
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
32
CR2: 0000000020002840 CR3: 00000000247e6000 CR4: 00000000003526f0
33
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
34
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
35
Call Trace:
36
<TASK>
37
genl_family_rcv_msg_doit net/netlink/genetlink.c:1115 [inline]
38
genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
39
genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1210
40
netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2542
41
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
42
netlink_unicast_kernel net/netlink/af_netlink.c:1321 [inline]
43
netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1347
44
netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1891
45
sock_sendmsg_nosec net/socket.c:711 [inline]
46
__sock_sendmsg+0x221/0x270 net/socket.c:726
47
____sys_sendmsg+0x52a/0x7e0 net/socket.c:2583
48
___sys_sendmsg net/socket.c:2637 [inline]
49
__sys_sendmsg+0x269/0x350 net/socket.c:2669
50
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
51
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
52
entry_SYSCALL_64_after_hwframe+0x77/0x7f
53
RIP: 0033:0x7f5fe8785d29
54
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
55
RSP: 002b:00007fff571f5558 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
56
RAX: ffffffffffffffda RBX: 00007f5fe8975fa0 RCX: 00007f5fe8785d29
57
RDX: 0000000000000000 RSI: 0000000020000480 RDI: 0000000000000007
58
RBP: 00007f5fe8801b08 R08: 0000000000000000 R09: 0000000000000000
59
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
60
R13: 00007f5fe8975fa0 R14: 00007f5fe8975fa0 R15: 00000000000011f4
61
</TASK>
62
63
Here, syzbot managed to set the 'fullmesh' flag on an 'implicit' and
64
used -- according to 'id_avail_bitmap' -- endpoint, causing the PM to
65
try decrement the local_addr_used counter which is only incremented for
66
the 'subflow' endpoint.
67
68
Note that 'no type' endpoints -- not 'subflow', 'signal', 'implicit' --
69
are fine, because their ID will not be marked as used in the 'id_avail'
70
bitmap, and setting 'fullmesh' can help forcing the creation of subflow
71
when receiving an ADD_ADDR.
72
73
Fixes: 73c762c1f07d ("mptcp: set fullmesh flag in pm_netlink")
74
Reported-by: syzbot+cd16e79c1e45f3fe0377@syzkaller.appspotmail.com
75
Closes: https://lore.kernel.org/6786ac51.050a0220.216c54.00a6.GAE@google.com
76
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/540
77
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
9
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
78
---
10
---
79
net/mptcp/pm_netlink.c | 3 ++-
11
Changes in v3:
80
1 file changed, 2 insertions(+), 1 deletion(-)
12
- the previous patch 1 has been applied
13
- patch 1: add Mat's RvB tag
14
- patch 2: clarify the modifications: new comment and description (Mat)
15
- Link to v2: https://lore.kernel.org/r/20250117-mptcp-issue-540-v2-0-a194740fb380@kernel.org
81
16
82
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
17
Changes in v2:
83
index XXXXXXX..XXXXXXX 100644
18
- patch 1: check the two flags in one instruction. (Paolo)
84
--- a/net/mptcp/pm_netlink.c
19
- patch 2-3: new patches, for next.
85
+++ b/net/mptcp/pm_netlink.c
20
- Link to v1: https://lore.kernel.org/r/20250116-mptcp-issue-540-v1-1-b0ba2c126eb8@kernel.org
86
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
87
        return -EINVAL;
88
    }
89
    if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
90
-     (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
91
+     ((entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) ||
92
+     (entry->flags & MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
93
        spin_unlock_bh(&pernet->lock);
94
        NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
95
        return -EINVAL;
96
21
97
---
22
---
98
base-commit: 9f10cc97addfb3a81b4aaf4b13f59b36b5bcc7a6
23
Matthieu Baerts (NGI0) (2):
24
mptcp: pm: remove unused ret value to set flags
25
mptcp: pm: change to fullmesh only for 'subflow'
26
27
net/mptcp/pm_netlink.c | 24 ++++++++++++------------
28
1 file changed, 12 insertions(+), 12 deletions(-)
29
---
30
base-commit: 8c4d55afa2c3c2391af5d975ab3b1a5bcc140101
99
change-id: 20250116-mptcp-issue-540-49d1db3346a9
31
change-id: 20250116-mptcp-issue-540-49d1db3346a9
100
32
101
Best regards,
33
Best regards,
102
--
34
--
103
Matthieu Baerts (NGI0) <matttbe@kernel.org>
35
Matthieu Baerts (NGI0) <matttbe@kernel.org>
diff view generated by jsdifflib
New patch
1
The returned value is not used, it can then be dropped.
1
2
3
Reviewed-by: Mat Martineau <martineau@kernel.org>
4
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
5
---
6
net/mptcp/pm_netlink.c | 10 ++++------
7
1 file changed, 4 insertions(+), 6 deletions(-)
8
9
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
10
index XXXXXXX..XXXXXXX 100644
11
--- a/net/mptcp/pm_netlink.c
12
+++ b/net/mptcp/pm_netlink.c
13
@@ -XXX,XX +XXX,XX @@ static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
14
    spin_unlock_bh(&msk->pm.lock);
15
}
16
17
-static int mptcp_nl_set_flags(struct net *net,
18
-             struct mptcp_addr_info *addr,
19
-             u8 bkup, u8 changed)
20
+static void mptcp_nl_set_flags(struct net *net, struct mptcp_addr_info *addr,
21
+             u8 bkup, u8 changed)
22
{
23
    long s_slot = 0, s_num = 0;
24
    struct mptcp_sock *msk;
25
-    int ret = -EINVAL;
26
27
    while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
28
        struct sock *sk = (struct sock *)msk;
29
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
30
31
        lock_sock(sk);
32
        if (changed & MPTCP_PM_ADDR_FLAG_BACKUP)
33
-            ret = mptcp_pm_nl_mp_prio_send_ack(msk, addr, NULL, bkup);
34
+            mptcp_pm_nl_mp_prio_send_ack(msk, addr, NULL, bkup);
35
        if (changed & MPTCP_PM_ADDR_FLAG_FULLMESH)
36
            mptcp_pm_nl_fullmesh(msk, addr);
37
        release_sock(sk);
38
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
39
        cond_resched();
40
    }
41
42
-    return ret;
43
+    return;
44
}
45
46
int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
47
48
--
49
2.47.1
diff view generated by jsdifflib
New patch
1
If an endpoint doesn't have the 'subflow' endpoint -- in fact, has no
2
type, so not 'subflow', 'signal', nor 'implicit' -- there are then no
3
subflows created from this local endpoint to at least the initial
4
destination address. In this case, no need to call mptcp_pm_nl_fullmesh()
5
which is there to recreate the subflows to reflect the new value of the
6
fullmesh attribute.
1
7
8
Similarly, there is then no need to iterate over all connections to do
9
nothing, if only the 'fullmesh' flag has been changed, and the endpoint
10
doesn't have the 'subflow' one. So stop early when dealing with this
11
specific case.
12
13
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
14
---
15
Notes:
16
v2:
17
- clarify the modifications: new comment and updated description (Mat)
18
---
19
net/mptcp/pm_netlink.c | 16 +++++++++-------
20
1 file changed, 9 insertions(+), 7 deletions(-)
21
22
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
23
index XXXXXXX..XXXXXXX 100644
24
--- a/net/mptcp/pm_netlink.c
25
+++ b/net/mptcp/pm_netlink.c
26
@@ -XXX,XX +XXX,XX @@ static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
27
}
28
29
static void mptcp_nl_set_flags(struct net *net, struct mptcp_addr_info *addr,
30
-             u8 bkup, u8 changed)
31
+             u8 flags, u8 changed)
32
{
33
+    u8 is_subflow = !!(flags & MPTCP_PM_ADDR_FLAG_SUBFLOW);
34
+    u8 bkup = !!(flags & MPTCP_PM_ADDR_FLAG_BACKUP);
35
    long s_slot = 0, s_num = 0;
36
    struct mptcp_sock *msk;
37
38
+    if (changed == MPTCP_PM_ADDR_FLAG_FULLMESH && !is_subflow)
39
+        return;
40
+
41
    while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
42
        struct sock *sk = (struct sock *)msk;
43
44
@@ -XXX,XX +XXX,XX @@ static void mptcp_nl_set_flags(struct net *net, struct mptcp_addr_info *addr,
45
        lock_sock(sk);
46
        if (changed & MPTCP_PM_ADDR_FLAG_BACKUP)
47
            mptcp_pm_nl_mp_prio_send_ack(msk, addr, NULL, bkup);
48
-        if (changed & MPTCP_PM_ADDR_FLAG_FULLMESH)
49
+        /* Subflows will only be recreated if the SUBFLOW flag is set */
50
+        if (is_subflow && (changed & MPTCP_PM_ADDR_FLAG_FULLMESH))
51
            mptcp_pm_nl_fullmesh(msk, addr);
52
        release_sock(sk);
53
54
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
55
    struct mptcp_pm_addr_entry *entry;
56
    struct pm_nl_pernet *pernet;
57
    u8 lookup_by_id = 0;
58
-    u8 bkup = 0;
59
60
    pernet = pm_nl_get_pernet(net);
61
62
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
63
        }
64
    }
65
66
-    if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP)
67
-        bkup = 1;
68
-
69
    spin_lock_bh(&pernet->lock);
70
    entry = lookup_by_id ? __lookup_addr_by_id(pernet, local->addr.id) :
71
             __lookup_addr(pernet, &local->addr);
72
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
73
    *local = *entry;
74
    spin_unlock_bh(&pernet->lock);
75
76
-    mptcp_nl_set_flags(net, &local->addr, bkup, changed);
77
+    mptcp_nl_set_flags(net, &local->addr, entry->flags, changed);
78
    return 0;
79
}
80
81
82
--
83
2.47.1
diff view generated by jsdifflib