FAILED: patch "[PATCH] mptcp: pm: avoid possible UaF when selecting endp" failed to apply to 6.1-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 48e50dcbcbaaf713d82bf2da5c16aeced94ad07d
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024082656-bamboo-skinless-9366@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
Possible dependencies:
48e50dcbcbaa ("mptcp: pm: avoid possible UaF when selecting endp") 85df533a787b ("mptcp: pm: do not ignore 'subflow' if 'signal' flag is also set") cd7c957f936f ("mptcp: pm: don't try to create sf if alloc failed") c95eb32ced82 ("mptcp: pm: reduce indentation blocks")528cb5f2a1e8 ("mptcp: pass addr to mptcp_pm_alloc_anno_list") 77e4b94a3de6 ("mptcp: update userspace pm infos")24430f8bf516 ("mptcp: add address into userspace pm list") b9d69db87fb7 ("mptcp: let the in-kernel PM use mixed IPv4 and IPv6 addresses") fb00ee4f3343 ("mptcp: netlink: respect v4/v6-only sockets") 80638684e840 ("mptcp: get sk from msk directly")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 48e50dcbcbaaf713d82bf2da5c16aeced94ad07d Mon Sep 17 00:00:00 2001
From: "Matthieu Baerts (NGI0)" <[email protected]>
Date: Mon, 19 Aug 2024 21:45:32 +0200
Subject: [PATCH] mptcp: pm: avoid possible UaF when selecting endp
select_local_address() and select_signal_address() both select an endpoint entry from the list inside an RCU protected section, but return a reference to it, to be read later on. If the entry is dereferenced after the RCU unlock, reading info could cause a Use-after-Free.
A simple solution is to copy the required info while inside the RCU protected section to avoid any risk of UaF later. The address ID might need to be modified later to handle the ID0 case later, so a copy seems OK to deal with.
net/mptcp/pm_netlink.c +34 -30
@@ -143,11 +143,13 @@ static bool lookup_subflow_by_daddr(const struct list_head *list,return false; } -static struct mptcp_pm_addr_entry * +static bool select_local_address (const struct pm_nl_pernet *pernet, -const struct mptcp_sock *msk) +const struct mptcp_sock *msk, +struct mptcp_pm_addr_entry *new_entry) { -struct mptcp_pm_addr_entry *entry, *ret =NULL ; +struct mptcp_pm_addr_entry *entry; +bool found = false;msk_owned_by_me (msk);
@@ -159,17 +161,21 @@ select_local_address(const struct pm_nl_pernet *pernet,if (!test_bit (entry->addr.id, msk->pm.id_avail_bitmap))continue ; - ret = entry; + *new_entry = *entry; + found = true;break ; }rcu_read_unlock (); -return ret; + +return found; } -static struct mptcp_pm_addr_entry * -select_signal_address (struct pm_nl_pernet *pernet,const struct mptcp_sock *msk) +static bool +select_signal_address (struct pm_nl_pernet *pernet,const struct mptcp_sock *msk, +struct mptcp_pm_addr_entry *new_entry) { -struct mptcp_pm_addr_entry *entry, *ret =NULL ; +struct mptcp_pm_addr_entry *entry; +bool found = false;rcu_read_lock ();/* do not keep any additional per socket state, just signal
@@ -184,11 +190,13 @@ select_signal_address(struct pm_nl_pernet *pernet, const struct mptcp_sock *msk)if (!(entry->flags &MPTCP_PM_ADDR_FLAG_SIGNAL ))continue ; - ret = entry; + *new_entry = *entry; + found = true;break ; }rcu_read_unlock (); -return ret; + +return found; }unsigned int mptcp_pm_get_add_addr_signal_max (const struct mptcp_sock *msk)
@@ -512,9 +520,10 @@ __lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info)static void mptcp_pm_create_subflow_or_signal_addr (struct mptcp_sock *msk) { -struct mptcp_pm_addr_entry *local, *signal_and_subflow =NULL ;struct sock *sk = (struct sock *)msk; +struct mptcp_pm_addr_entry local;unsigned int add_addr_signal_max; +bool signal_and_subflow = false;unsigned int local_addr_max;struct pm_nl_pernet *pernet;unsigned int subflows_max;
@@ -565,23 +574,22 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)if (msk->pm.addr_signal &BIT (MPTCP_ADD_ADDR_SIGNAL ))return ; - local =select_signal_address (pernet, msk); -if (!local) +if (!select_signal_address (pernet, msk, &local)) goto subflow;/* If the alloc fails, we are on memory pressure, not worth * continuing, and trying to create subflows. */ -if (!mptcp_pm_alloc_anno_list (msk, &local->addr)) +if (!mptcp_pm_alloc_anno_list (msk, &local.addr))return ; -__clear_bit (local->addr.id, msk->pm.id_avail_bitmap); +__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); msk->pm.add_addr_signaled++; -mptcp_pm_announce_addr (msk, &local->addr, false); +mptcp_pm_announce_addr (msk, &local.addr, false);mptcp_pm_nl_addr_send_ack (msk); -if (local->flags &MPTCP_PM_ADDR_FLAG_SUBFLOW ) - signal_and_subflow = local; +if (local.flags &MPTCP_PM_ADDR_FLAG_SUBFLOW ) + signal_and_subflow = true; } subflow:
@@ -592,26 +600,22 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)bool fullmesh;int i, nr; -if (signal_and_subflow) { - local = signal_and_subflow; - signal_and_subflow =NULL ; - }else { - local =select_local_address (pernet, msk); -if (!local) -break ; - } +if (signal_and_subflow) + signal_and_subflow = false; +else if (!select_local_address (pernet, msk, &local)) +break ; - fullmesh = !!(local->flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); + fullmesh = !!(local.flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); msk->pm.local_addr_used++; -__clear_bit (local->addr.id, msk->pm.id_avail_bitmap); - nr =fill_remote_addresses_vec (msk, &local->addr, fullmesh, addrs); +__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); + nr =fill_remote_addresses_vec (msk, &local.addr, fullmesh, addrs);if (nr ==0 )continue ;spin_unlock_bh (&msk->pm.lock);for (i =0 ; i < nr; i++) -__mptcp_subflow_connect (sk, &local->addr, &addrs[i]); +__mptcp_subflow_connect (sk, &local.addr, &addrs[i]);spin_lock_bh (&msk->pm.lock); }mptcp_pm_nl_check_work_pending (msk);
Matthieu Baerts (NGI0)
maintainerPATCH 1/1 mptcp: pm: avoid possible UaF when selecting endp
commit 48e50dcbcbaaf713d82bf2da5c16aeced94ad07d upstream.
select_local_address() and select_signal_address() both select an endpoint entry from the list inside an RCU protected section, but return a reference to it, to be read later on. If the entry is dereferenced after the RCU unlock, reading info could cause a Use-after-Free.
A simple solution is to copy the required info while inside the RCU protected section to avoid any risk of UaF later. The address ID might need to be modified later to handle the ID0 case later, so a copy seems OK to deal with.
[ Conflicts in pm_netlink.c, because the context has been modified in
commit b9d69db87fb7 ("mptcp: let the in-kernel PM use mixed IPv4 and IPv6 addresses"), which is not a candidate for the backports. The same modifications have been applied in this version. The conflict in mptcp_pm_create_subflow_or_signal_addr() has been resolved by taking the newer version, which skip a lock if it is not needed. ]
--- net/mptcp/pm_netlink.c | 68 +++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 31 deletions(-)
net/mptcp/pm_netlink.c +37 -31
@@ -150,12 +150,14 @@ static bool lookup_subflow_by_daddr(const struct list_head *list,return false; } -static struct mptcp_pm_addr_entry * +static bool select_local_address (const struct pm_nl_pernet *pernet, -const struct mptcp_sock *msk) +const struct mptcp_sock *msk, +struct mptcp_pm_addr_entry *new_entry) {const struct sock *sk = (const struct sock *)msk; -struct mptcp_pm_addr_entry *entry, *ret =NULL ; +struct mptcp_pm_addr_entry *entry; +bool found = false;msk_owned_by_me (msk);
@@ -177,17 +179,21 @@ select_local_address(const struct pm_nl_pernet *pernet,continue ; } - ret = entry; + *new_entry = *entry; + found = true;break ; }rcu_read_unlock (); -return ret; + +return found; } -static struct mptcp_pm_addr_entry * -select_signal_address (struct pm_nl_pernet *pernet,const struct mptcp_sock *msk) +static bool +select_signal_address (struct pm_nl_pernet *pernet,const struct mptcp_sock *msk, +struct mptcp_pm_addr_entry *new_entry) { -struct mptcp_pm_addr_entry *entry, *ret =NULL ; +struct mptcp_pm_addr_entry *entry; +bool found = false;rcu_read_lock ();/* do not keep any additional per socket state, just signal
@@ -202,11 +208,13 @@ select_signal_address(struct pm_nl_pernet *pernet, const struct mptcp_sock *msk)if (!(entry->flags &MPTCP_PM_ADDR_FLAG_SIGNAL ))continue ; - ret = entry; + *new_entry = *entry; + found = true;break ; }rcu_read_unlock (); -return ret; + +return found; }unsigned int mptcp_pm_get_add_addr_signal_max (const struct mptcp_sock *msk)
@@ -527,9 +535,10 @@ __lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info,static void mptcp_pm_create_subflow_or_signal_addr (struct mptcp_sock *msk) { -struct mptcp_pm_addr_entry *local, *signal_and_subflow =NULL ;struct sock *sk = (struct sock *)msk; +struct mptcp_pm_addr_entry local;unsigned int add_addr_signal_max; +bool signal_and_subflow = false;unsigned int local_addr_max;struct pm_nl_pernet *pernet;unsigned int subflows_max;
@@ -580,23 +589,22 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)if (msk->pm.addr_signal &BIT (MPTCP_ADD_ADDR_SIGNAL ))return ; - local =select_signal_address (pernet, msk); -if (!local) +if (!select_signal_address (pernet, msk, &local)) goto subflow;/* If the alloc fails, we are on memory pressure, not worth * continuing, and trying to create subflows. */ -if (!mptcp_pm_alloc_anno_list (msk, &local->addr)) +if (!mptcp_pm_alloc_anno_list (msk, &local.addr))return ; -__clear_bit (local->addr.id, msk->pm.id_avail_bitmap); +__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); msk->pm.add_addr_signaled++; -mptcp_pm_announce_addr (msk, &local->addr, false); +mptcp_pm_announce_addr (msk, &local.addr, false);mptcp_pm_nl_addr_send_ack (msk); -if (local->flags &MPTCP_PM_ADDR_FLAG_SUBFLOW ) - signal_and_subflow = local; +if (local.flags &MPTCP_PM_ADDR_FLAG_SUBFLOW ) + signal_and_subflow = true; } subflow:
@@ -607,24 +615,22 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)bool fullmesh;int i, nr; -if (signal_and_subflow) { - local = signal_and_subflow; - signal_and_subflow =NULL ; - }else { - local =select_local_address (pernet, msk); -if (!local) -break ; - } +if (signal_and_subflow) + signal_and_subflow = false; +else if (!select_local_address (pernet, msk, &local)) +break ; - fullmesh = !!(local->flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); + fullmesh = !!(local.flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); msk->pm.local_addr_used++; - nr =fill_remote_addresses_vec (msk, &local->addr, fullmesh, addrs); -if (nr) -__clear_bit (local->addr.id, msk->pm.id_avail_bitmap); +__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); + nr =fill_remote_addresses_vec (msk, &local.addr, fullmesh, addrs); +if (nr ==0 ) +continue ; +spin_unlock_bh (&msk->pm.lock);for (i =0 ; i < nr; i++) -__mptcp_subflow_connect (sk, &local->addr, &addrs[i]); +__mptcp_subflow_connect (sk, &local.addr, &addrs[i]);spin_lock_bh (&msk->pm.lock); }mptcp_pm_nl_check_work_pending (msk);
Signature · 1 line
2.45.2
Matthieu Baerts (NGI0)
maintainercover Backport of "mptcp: pm: reuse ID 0 after delete and re-add" and more
After having applied the patch "mptcp: pm: avoid possible UaF when selecting endp" in v6.1 with the adaptations proposed in the parent message [1], the two following patches can be applied without conflicts:
- 8b8ed1b429f8 ("mptcp: pm: reuse ID 0 after delete and re-add")
- 9366922adc6a ("mptcp: pm: fix ID 0 endp usage after multiple re-creations")
[1] https://lore.kernel.org/[email protected]
Matthieu Baerts (NGI0) (2):
mptcp: pm: reuse ID 0 after delete and re-add
mptcp: pm: fix ID 0 endp usage after multiple re-creations
net/mptcp/pm_netlink.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
Signature · 1 line
2.45.2
Matthieu Baerts (NGI0)
maintainerPATCH 1/2 mptcp: pm: reuse ID 0 after delete and re-add
In reply to Matthieu Baerts (NGI0)commit 8b8ed1b429f8fa7ebd5632555e7b047bc0620075 upstream.
When the endpoint used by the initial subflow is removed and re-added later, the PM has to force the ID 0, it is a special case imposed by the MPTCP specs.
Note that the endpoint should then need to be re-added reusing the same ID.
--- net/mptcp/pm_netlink.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
net/mptcp/pm_netlink.c +10 -0
@@ -600,6 +600,11 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); msk->pm.add_addr_signaled++; + +/* Special case for ID0: set the correct ID */ +if (local.addr.id == msk->mpc_endpoint_id) + local.addr.id =0 ; +mptcp_pm_announce_addr (msk, &local.addr, false);mptcp_pm_nl_addr_send_ack (msk);
@@ -624,6 +629,11 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) msk->pm.local_addr_used++;__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); + +/* Special case for ID0: set the correct ID */ +if (local.addr.id == msk->mpc_endpoint_id) + local.addr.id =0 ; + nr =fill_remote_addresses_vec (msk, &local.addr, fullmesh, addrs);if (nr ==0 )continue ;
Signature · 1 line
2.45.2
Re: [PATCH 6.1.y 1/2] mptcp: pm: reuse ID 0 after delete and re-add
Quoted 2 lines · On Wed, Sep 04, 2024 at 01:03:08PM +0200, Matthieu Baerts (NGI0) wrote:
commit
8b8ed1b429f8fa7ebd5632555e7b047bc0620075upstream.
Applied
Patch "mptcp: pm: reuse ID 0 after delete and re-add" has been added to the 6.1-stable tree
In reply to Matthieu Baerts (NGI0)This is a note to let you know that I've just added the patch titled
mptcp: pm: reuse ID 0 after delete and re-add
to the 6.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mptcp-pm-reuse-id-0-after-delete-and-re-add.patch
and it can be found in the queue-6.1 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
From [email protected] Wed Sep 4 13:04:05 2024
From: "Matthieu Baerts (NGI0)" <[email protected]>
Date: Wed, 4 Sep 2024 13:03:08 +0200
Subject: mptcp: pm: reuse ID 0 after delete and re-add
To: [email protected], [email protected], [email protected]
Cc: "Matthieu Baerts (NGI0)" <[email protected]>, Mat Martineau <[email protected]>, Paolo Abeni <[email protected]>
Message-ID: <[email protected]>
From: "Matthieu Baerts (NGI0)" <[email protected]>
commit 8b8ed1b429f8fa7ebd5632555e7b047bc0620075 upstream.
When the endpoint used by the initial subflow is removed and re-added later, the PM has to force the ID 0, it is a special case imposed by the MPTCP specs.
Note that the endpoint should then need to be re-added reusing the same ID.
--- net/mptcp/pm_netlink.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
net/mptcp/pm_netlink.c +10 -0
@@ -600,6 +600,11 @@ static void mptcp_pm_create_subflow_or_s__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); msk->pm.add_addr_signaled++; + +/* Special case for ID0: set the correct ID */ +if (local.addr.id == msk->mpc_endpoint_id) + local.addr.id =0 ; +mptcp_pm_announce_addr (msk, &local.addr, false);mptcp_pm_nl_addr_send_ack (msk);
@@ -624,6 +629,11 @@ subflow: msk->pm.local_addr_used++;__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); + +/* Special case for ID0: set the correct ID */ +if (local.addr.id == msk->mpc_endpoint_id) + local.addr.id =0 ; + nr =fill_remote_addresses_vec (msk, &local.addr, fullmesh, addrs);if (nr ==0 )continue ;
Patches currently in stable-queue which might be from [email protected] are
queue-6.1/mptcp-pm-fix-rm_addr-id-for-the-initial-subflow.patch queue-6.1/selftests-mptcp-join-validate-fullmesh-endp-on-1st-sf.patch queue-6.1/mptcp-pm-fix-id-0-endp-usage-after-multiple-re-creations.patch queue-6.1/mptcp-make-pm_remove_addrs_and_subflows-static.patch queue-6.1/mptcp-pm-avoid-possible-uaf-when-selecting-endp.patch queue-6.1/mptcp-pm-reuse-id-0-after-delete-and-re-add.patch queue-6.1/selftests-mptcp-join-check-re-adding-init-endp-with-id.patch queue-6.1/selftests-mptcp-join-test-for-flush-re-add-endpoints.patch queue-6.1/selftests-mptcp-add-explicit-test-case-for-remove-re.patch queue-6.1/selftests-mptcp-add-explicit-test-case-for-remove-readd.patch queue-6.1/selftests-mptcp-join-check-re-adding-init-endp-with-.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-closed-subflow.patch queue-6.1/mptcp-pm-fullmesh-select-the-right-id-later.patch queue-6.1/mptcp-pr_debug-add-missing-n-at-the-end.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-unused-add.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-unused-add_addr.patch
Matthieu Baerts (NGI0)
maintainerPATCH 2/2 mptcp: pm: fix ID 0 endp usage after multiple re-creations
In reply to Matthieu Baerts (NGI0)commit 9366922adc6a71378ca01f898c41be295309f044 upstream.
'local_addr_used' and 'add_addr_accepted' are decremented for addresses not related to the initial subflow (ID0), because the source and destination addresses of the initial subflows are known from the
beginning: they don't count as "additional local address being used" or "ADD_ADDR being accepted".
It is then required not to increment them when the entrypoint used by the initial subflow is removed and re-added during a connection. Without this modification, this entrypoint cannot be removed and re-added more than once.
--- net/mptcp/pm_netlink.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
net/mptcp/pm_netlink.c +5 -2
@@ -627,12 +627,13 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) fullmesh = !!(local.flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); - msk->pm.local_addr_used++;__clear_bit (local.addr.id, msk->pm.id_avail_bitmap);/* Special case for ID0: set the correct ID */ if (local.addr.id == msk->mpc_endpoint_id) local.addr.id =0 ; +else /* local_addr_used is not decr for ID 0 */ + msk->pm.local_addr_used++; nr =fill_remote_addresses_vec (msk, &local.addr, fullmesh, addrs);if (nr ==0 )
@@ -758,7 +759,9 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)spin_lock_bh (&msk->pm.lock);if (sf_created) { - msk->pm.add_addr_accepted++; +/* add_addr_accepted is not decr for ID 0 */ +if (remote.id) + msk->pm.add_addr_accepted++;if (msk->pm.add_addr_accepted >= add_addr_accept_max || msk->pm.subflows >= subflows_max)WRITE_ONCE (msk->pm.accept_addr, false);
Signature · 1 line
2.45.2
Re: [PATCH 6.1.y 2/2] mptcp: pm: fix ID 0 endp usage after multiple re-creations
Quoted 2 lines · On Wed, Sep 04, 2024 at 01:03:09PM +0200, Matthieu Baerts (NGI0) wrote:
commit
9366922adc6a71378ca01f898c41be295309f044upstream.
Applied
Patch "mptcp: pm: fix ID 0 endp usage after multiple re-creations" has been added to the 6.1-stable tree
In reply to Matthieu Baerts (NGI0)This is a note to let you know that I've just added the patch titled
mptcp: pm: fix ID 0 endp usage after multiple re-creations
to the 6.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mptcp-pm-fix-id-0-endp-usage-after-multiple-re-creations.patch
and it can be found in the queue-6.1 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
From [email protected] Wed Sep 4 13:04:07 2024
From: "Matthieu Baerts (NGI0)" <[email protected]>
Date: Wed, 4 Sep 2024 13:03:09 +0200
Subject: mptcp: pm: fix ID 0 endp usage after multiple re-creations
To: [email protected], [email protected], [email protected]
Cc: "Matthieu Baerts (NGI0)" <[email protected]>, "Arınç ÜNAL" <[email protected]>, [email protected], "Mat Martineau" <[email protected]>, "Paolo Abeni" <[email protected]>
Message-ID: <[email protected]>
From: "Matthieu Baerts (NGI0)" <[email protected]>
commit 9366922adc6a71378ca01f898c41be295309f044 upstream.
'local_addr_used' and 'add_addr_accepted' are decremented for addresses not related to the initial subflow (ID0), because the source and destination addresses of the initial subflows are known from the
beginning: they don't count as "additional local address being used" or "ADD_ADDR being accepted".
It is then required not to increment them when the entrypoint used by the initial subflow is removed and re-added during a connection. Without this modification, this entrypoint cannot be removed and re-added more than once.
--- net/mptcp/pm_netlink.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
net/mptcp/pm_netlink.c +5 -2
@@ -627,12 +627,13 @@ subflow: fullmesh = !!(local.flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); - msk->pm.local_addr_used++;__clear_bit (local.addr.id, msk->pm.id_avail_bitmap);/* Special case for ID0: set the correct ID */ if (local.addr.id == msk->mpc_endpoint_id) local.addr.id =0 ; +else /* local_addr_used is not decr for ID 0 */ + msk->pm.local_addr_used++; nr =fill_remote_addresses_vec (msk, &local.addr, fullmesh, addrs);if (nr ==0 )
@@ -758,7 +759,9 @@ static void mptcp_pm_nl_add_addr_receivespin_lock_bh (&msk->pm.lock);if (sf_created) { - msk->pm.add_addr_accepted++; +/* add_addr_accepted is not decr for ID 0 */ +if (remote.id) + msk->pm.add_addr_accepted++;if (msk->pm.add_addr_accepted >= add_addr_accept_max || msk->pm.subflows >= subflows_max)WRITE_ONCE (msk->pm.accept_addr, false);
Patches currently in stable-queue which might be from [email protected] are
queue-6.1/mptcp-pm-fix-rm_addr-id-for-the-initial-subflow.patch queue-6.1/selftests-mptcp-join-validate-fullmesh-endp-on-1st-sf.patch queue-6.1/mptcp-pm-fix-id-0-endp-usage-after-multiple-re-creations.patch queue-6.1/mptcp-make-pm_remove_addrs_and_subflows-static.patch queue-6.1/mptcp-pm-avoid-possible-uaf-when-selecting-endp.patch queue-6.1/mptcp-pm-reuse-id-0-after-delete-and-re-add.patch queue-6.1/selftests-mptcp-join-check-re-adding-init-endp-with-id.patch queue-6.1/selftests-mptcp-join-test-for-flush-re-add-endpoints.patch queue-6.1/selftests-mptcp-add-explicit-test-case-for-remove-re.patch queue-6.1/selftests-mptcp-add-explicit-test-case-for-remove-readd.patch queue-6.1/selftests-mptcp-join-check-re-adding-init-endp-with-.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-closed-subflow.patch queue-6.1/mptcp-pm-fullmesh-select-the-right-id-later.patch queue-6.1/mptcp-pr_debug-add-missing-n-at-the-end.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-unused-add.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-unused-add_addr.patch
Re: [PATCH 6.1.y] mptcp: pm: avoid possible UaF when selecting endp
In reply to Matthieu Baerts (NGI0)Quoted 2 lines · On Wed, Sep 04, 2024 at 12:57:22PM +0200, Matthieu Baerts (NGI0) wrote:
commit
48e50dcbcbaaf713d82bf2da5c16aeced94ad07dupstream.
Applied
Patch "mptcp: pm: avoid possible UaF when selecting endp" has been added to the 6.1-stable tree
In reply to Matthieu Baerts (NGI0)This is a note to let you know that I've just added the patch titled
mptcp: pm: avoid possible UaF when selecting endp
to the 6.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mptcp-pm-avoid-possible-uaf-when-selecting-endp.patch
and it can be found in the queue-6.1 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
From [email protected] Wed Sep 4 12:58:37 2024
From: "Matthieu Baerts (NGI0)" <[email protected]>
Date: Wed, 4 Sep 2024 12:57:22 +0200
Subject: mptcp: pm: avoid possible UaF when selecting endp
To: [email protected], [email protected]
Cc: MPTCP Upstream <[email protected]>, "Matthieu Baerts (NGI0)" <[email protected]>, Paolo Abeni <[email protected]>, Mat Martineau <[email protected]>, Jakub Kicinski <[email protected]>
Message-ID: <[email protected]>
From: "Matthieu Baerts (NGI0)" <[email protected]>
commit 48e50dcbcbaaf713d82bf2da5c16aeced94ad07d upstream.
select_local_address() and select_signal_address() both select an endpoint entry from the list inside an RCU protected section, but return a reference to it, to be read later on. If the entry is dereferenced after the RCU unlock, reading info could cause a Use-after-Free.
A simple solution is to copy the required info while inside the RCU protected section to avoid any risk of UaF later. The address ID might need to be modified later to handle the ID0 case later, so a copy seems OK to deal with.
[ Conflicts in pm_netlink.c, because the context has been modified in
commit b9d69db87fb7 ("mptcp: let the in-kernel PM use mixed IPv4 and IPv6 addresses"), which is not a candidate for the backports. The same modifications have been applied in this version. The conflict in mptcp_pm_create_subflow_or_signal_addr() has been resolved by taking the newer version, which skip a lock if it is not needed. ]
--- net/mptcp/pm_netlink.c | 68 ++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 31 deletions(-)
net/mptcp/pm_netlink.c +37 -31
@@ -150,12 +150,14 @@ static bool lookup_subflow_by_daddr(consreturn false; } -static struct mptcp_pm_addr_entry * +static bool select_local_address (const struct pm_nl_pernet *pernet, -const struct mptcp_sock *msk) +const struct mptcp_sock *msk, +struct mptcp_pm_addr_entry *new_entry) {const struct sock *sk = (const struct sock *)msk; -struct mptcp_pm_addr_entry *entry, *ret =NULL ; +struct mptcp_pm_addr_entry *entry; +bool found = false;msk_owned_by_me (msk);
@@ -177,17 +179,21 @@ select_local_address(const struct pm_nl_continue ; } - ret = entry; + *new_entry = *entry; + found = true;break ; }rcu_read_unlock (); -return ret; + +return found; } -static struct mptcp_pm_addr_entry * -select_signal_address (struct pm_nl_pernet *pernet,const struct mptcp_sock *msk) +static bool +select_signal_address (struct pm_nl_pernet *pernet,const struct mptcp_sock *msk, +struct mptcp_pm_addr_entry *new_entry) { -struct mptcp_pm_addr_entry *entry, *ret =NULL ; +struct mptcp_pm_addr_entry *entry; +bool found = false;rcu_read_lock ();/* do not keep any additional per socket state, just signal
@@ -202,11 +208,13 @@ select_signal_address(struct pm_nl_perneif (!(entry->flags &MPTCP_PM_ADDR_FLAG_SIGNAL ))continue ; - ret = entry; + *new_entry = *entry; + found = true;break ; }rcu_read_unlock (); -return ret; + +return found; }unsigned int mptcp_pm_get_add_addr_signal_max (const struct mptcp_sock *msk)
@@ -527,9 +535,10 @@ __lookup_addr(struct pm_nl_pernet *pernestatic void mptcp_pm_create_subflow_or_signal_addr (struct mptcp_sock *msk) { -struct mptcp_pm_addr_entry *local, *signal_and_subflow =NULL ;struct sock *sk = (struct sock *)msk; +struct mptcp_pm_addr_entry local;unsigned int add_addr_signal_max; +bool signal_and_subflow = false;unsigned int local_addr_max;struct pm_nl_pernet *pernet;unsigned int subflows_max;
@@ -580,23 +589,22 @@ static void mptcp_pm_create_subflow_or_sif (msk->pm.addr_signal &BIT (MPTCP_ADD_ADDR_SIGNAL ))return ; - local =select_signal_address (pernet, msk); -if (!local) +if (!select_signal_address (pernet, msk, &local)) goto subflow;/* If the alloc fails, we are on memory pressure, not worth * continuing, and trying to create subflows. */ -if (!mptcp_pm_alloc_anno_list (msk, &local->addr)) +if (!mptcp_pm_alloc_anno_list (msk, &local.addr))return ; -__clear_bit (local->addr.id, msk->pm.id_avail_bitmap); +__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); msk->pm.add_addr_signaled++; -mptcp_pm_announce_addr (msk, &local->addr, false); +mptcp_pm_announce_addr (msk, &local.addr, false);mptcp_pm_nl_addr_send_ack (msk); -if (local->flags &MPTCP_PM_ADDR_FLAG_SUBFLOW ) - signal_and_subflow = local; +if (local.flags &MPTCP_PM_ADDR_FLAG_SUBFLOW ) + signal_and_subflow = true; } subflow:
@@ -607,24 +615,22 @@ subflow:bool fullmesh;int i, nr; -if (signal_and_subflow) { - local = signal_and_subflow; - signal_and_subflow =NULL ; - }else { - local =select_local_address (pernet, msk); -if (!local) -break ; - } +if (signal_and_subflow) + signal_and_subflow = false; +else if (!select_local_address (pernet, msk, &local)) +break ; - fullmesh = !!(local->flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); + fullmesh = !!(local.flags &MPTCP_PM_ADDR_FLAG_FULLMESH ); msk->pm.local_addr_used++; - nr =fill_remote_addresses_vec (msk, &local->addr, fullmesh, addrs); -if (nr) -__clear_bit (local->addr.id, msk->pm.id_avail_bitmap); +__clear_bit (local.addr.id, msk->pm.id_avail_bitmap); + nr =fill_remote_addresses_vec (msk, &local.addr, fullmesh, addrs); +if (nr ==0 ) +continue ; +spin_unlock_bh (&msk->pm.lock);for (i =0 ; i < nr; i++) -__mptcp_subflow_connect (sk, &local->addr, &addrs[i]); +__mptcp_subflow_connect (sk, &local.addr, &addrs[i]);spin_lock_bh (&msk->pm.lock); }mptcp_pm_nl_check_work_pending (msk);
Patches currently in stable-queue which might be from [email protected] are
queue-6.1/mptcp-pm-fix-rm_addr-id-for-the-initial-subflow.patch queue-6.1/selftests-mptcp-join-validate-fullmesh-endp-on-1st-sf.patch queue-6.1/mptcp-pm-fix-id-0-endp-usage-after-multiple-re-creations.patch queue-6.1/mptcp-make-pm_remove_addrs_and_subflows-static.patch queue-6.1/mptcp-pm-avoid-possible-uaf-when-selecting-endp.patch queue-6.1/mptcp-pm-reuse-id-0-after-delete-and-re-add.patch queue-6.1/selftests-mptcp-join-check-re-adding-init-endp-with-id.patch queue-6.1/selftests-mptcp-join-test-for-flush-re-add-endpoints.patch queue-6.1/selftests-mptcp-add-explicit-test-case-for-remove-re.patch queue-6.1/selftests-mptcp-add-explicit-test-case-for-remove-readd.patch queue-6.1/selftests-mptcp-join-check-re-adding-init-endp-with-.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-closed-subflow.patch queue-6.1/mptcp-pm-fullmesh-select-the-right-id-later.patch queue-6.1/mptcp-pr_debug-add-missing-n-at-the-end.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-unused-add.patch queue-6.1/selftests-mptcp-join-check-re-using-id-of-unused-add_addr.patch