MPTCP Linux Development
 help / color / mirror / Atom feed
From: <gregkh@linuxfoundation.org>
To: eadavis@qq.com,gregkh@linuxfoundation.org,kuba@kernel.org,matttbe@kernel.org,mptcp@lists.linux.dev,pabeni@redhat.com,syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Cc: <stable-commits@vger.kernel.org>
Subject: Patch "mptcp: pm: Fix uaf in __timer_delete_sync" has been added to the 5.10-stable tree
Date: Fri, 27 Sep 2024 10:25:13 +0200	[thread overview]
Message-ID: <2024092713-massive-friday-655f@gregkh> (raw)
In-Reply-To: <20240917072607.799536-8-matttbe@kernel.org>


This is a note to let you know that I've just added the patch titled

    mptcp: pm: Fix uaf in __timer_delete_sync

to the 5.10-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-uaf-in-__timer_delete_sync.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


From stable+bounces-76550-greg=kroah.com@vger.kernel.org Tue Sep 17 09:26:39 2024
From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
Date: Tue, 17 Sep 2024 09:26:11 +0200
Subject: mptcp: pm: Fix uaf in __timer_delete_sync
To: mptcp@lists.linux.dev, stable@vger.kernel.org, gregkh@linuxfoundation.org
Cc: Edward Adam Davis <eadavis@qq.com>, syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com, Matthieu Baerts <matttbe@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>
Message-ID: <20240917072607.799536-8-matttbe@kernel.org>

From: Edward Adam Davis <eadavis@qq.com>

There are two paths to access mptcp_pm_del_add_timer, result in a race
condition:

     CPU1				CPU2
     ====                               ====
     net_rx_action
     napi_poll                          netlink_sendmsg
     __napi_poll                        netlink_unicast
     process_backlog                    netlink_unicast_kernel
     __netif_receive_skb                genl_rcv
     __netif_receive_skb_one_core       netlink_rcv_skb
     NF_HOOK                            genl_rcv_msg
     ip_local_deliver_finish            genl_family_rcv_msg
     ip_protocol_deliver_rcu            genl_family_rcv_msg_doit
     tcp_v4_rcv                         mptcp_pm_nl_flush_addrs_doit
     tcp_v4_do_rcv                      mptcp_nl_remove_addrs_list
     tcp_rcv_established                mptcp_pm_remove_addrs_and_subflows
     tcp_data_queue                     remove_anno_list_by_saddr
     mptcp_incoming_options             mptcp_pm_del_add_timer
     mptcp_pm_del_add_timer             kfree(entry)

In remove_anno_list_by_saddr(running on CPU2), after leaving the critical
zone protected by "pm.lock", the entry will be released, which leads to the
occurrence of uaf in the mptcp_pm_del_add_timer(running on CPU1).

Keeping a reference to add_timer inside the lock, and calling
sk_stop_timer_sync() with this reference, instead of "entry->add_timer".

Move list_del(&entry->list) to mptcp_pm_del_add_timer and inside the pm lock,
do not directly access any members of the entry outside the pm lock, which
can avoid similar "entry->x" uaf.

Fixes: 00cfd77b9063 ("mptcp: retransmit ADD_ADDR when timeout")
Cc: stable@vger.kernel.org
Reported-and-tested-by: syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f3a31fb909db9b2a5c4d
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
Acked-by: Paolo Abeni <pabeni@redhat.com>
Link: https://patch.msgid.link/tencent_7142963A37944B4A74EF76CD66EA3C253609@qq.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit b4cd80b0338945a94972ac3ed54f8338d2da2076)
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/mptcp/pm_netlink.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -253,15 +253,21 @@ mptcp_pm_del_add_timer(struct mptcp_sock
 {
 	struct mptcp_pm_add_entry *entry;
 	struct sock *sk = (struct sock *)msk;
+	struct timer_list *add_timer = NULL;
 
 	spin_lock_bh(&msk->pm.lock);
 	entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
-	if (entry && (!check_id || entry->addr.id == addr->id))
+	if (entry && (!check_id || entry->addr.id == addr->id)) {
 		entry->retrans_times = ADD_ADDR_RETRANS_MAX;
+		add_timer = &entry->add_timer;
+	}
+	if (!check_id && entry)
+		list_del(&entry->list);
 	spin_unlock_bh(&msk->pm.lock);
 
-	if (entry && (!check_id || entry->addr.id == addr->id))
-		sk_stop_timer_sync(sk, &entry->add_timer);
+	/* no lock, because sk_stop_timer_sync() is calling del_timer_sync() */
+	if (add_timer)
+		sk_stop_timer_sync(sk, add_timer);
 
 	return entry;
 }
@@ -766,7 +772,6 @@ static bool remove_anno_list_by_saddr(st
 
 	entry = mptcp_pm_del_add_timer(msk, addr, false);
 	if (entry) {
-		list_del(&entry->list);
 		kfree(entry);
 		return true;
 	}


Patches currently in stable-queue which might be from matttbe@kernel.org are

queue-5.10/mptcp-pm-fix-uaf-in-__timer_delete_sync.patch
queue-5.10/mptcp-validate-id-when-stopping-the-add_addr-retransmit-timer.patch
queue-5.10/mptcp-export-lookup_anno_list_by_saddr.patch

      reply	other threads:[~2024-09-27  8:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <2024091330-reps-craftsman-ab67@gregkh>
2024-09-17  7:26 ` [PATCH 5.10.y 0/3] Backport of "mptcp: pm: Fix uaf in __timer_delete_sync" Matthieu Baerts (NGI0)
2024-09-27  8:15   ` Greg KH
2024-09-17  7:26 ` [PATCH 5.10.y 1/3] mptcp: export lookup_anno_list_by_saddr Matthieu Baerts (NGI0)
2024-09-27  8:25   ` Patch "mptcp: export lookup_anno_list_by_saddr" has been added to the 5.10-stable tree gregkh
2024-09-17  7:26 ` [PATCH 5.10.y 2/3] mptcp: validate 'id' when stopping the ADD_ADDR retransmit timer Matthieu Baerts (NGI0)
2024-09-27  8:25   ` Patch "mptcp: validate 'id' when stopping the ADD_ADDR retransmit timer" has been added to the 5.10-stable tree gregkh
2024-09-17  7:26 ` [PATCH 5.10.y 3/3] mptcp: pm: Fix uaf in __timer_delete_sync Matthieu Baerts (NGI0)
2024-09-27  8:25   ` gregkh [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2024092713-massive-friday-655f@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=eadavis@qq.com \
    --cc=kuba@kernel.org \
    --cc=matttbe@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=pabeni@redhat.com \
    --cc=stable-commits@vger.kernel.org \
    --cc=syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox