mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Edward Adam Davis <eadavis@qq.com>
To: matttbe@kernel.org
Cc: davem@davemloft.net, eadavis@qq.com, edumazet@google.com,
	geliang@kernel.org, kuba@kernel.org,
	linux-kernel@vger.kernel.org, martineau@kernel.org,
	mptcp@lists.linux.dev, netdev@vger.kernel.org, pabeni@redhat.com,
	syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com,
	syzkaller-bugs@googlegroups.com
Subject: [PATCH net v3] mptcp: pm: Fix uaf in __timer_delete_sync
Date: Thu,  5 Sep 2024 20:27:23 +0800	[thread overview]
Message-ID: <tencent_F85DEC5DED99554FB28DEF258F8DB8120D07@qq.com> (raw)
In-Reply-To: <e4a13002-f471-4951-9180-14f0f8b30bd2@kernel.org>

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".

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>
---
 net/mptcp/pm_netlink.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 3e4ad801786f..7ddb373cc6ad 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -329,17 +329,21 @@ struct mptcp_pm_add_entry *
 mptcp_pm_del_add_timer(struct mptcp_sock *msk,
 		       const struct mptcp_addr_info *addr, bool check_id)
 {
-	struct mptcp_pm_add_entry *entry;
 	struct sock *sk = (struct sock *)msk;
+	struct timer_list *add_timer = NULL;
+	struct mptcp_pm_add_entry *entry;
 
 	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;
+	}
 	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;
 }
@@ -1430,8 +1434,10 @@ static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
 
 	entry = mptcp_pm_del_add_timer(msk, addr, false);
 	if (entry) {
+		spin_lock_bh(&msk->pm.lock);
 		list_del(&entry->list);
 		kfree(entry);
+		spin_unlock_bh(&msk->pm.lock);
 		return true;
 	}
 
-- 
2.43.0


  reply	other threads:[~2024-09-05 12:32 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-03 13:13 [syzbot] [mptcp?] KASAN: slab-use-after-free Read " syzbot
2024-09-03 13:43 ` Edward Adam Davis
2024-09-03 14:09   ` syzbot
2024-09-03 13:52 ` Edward Adam Davis
2024-09-03 14:24   ` syzbot
2024-09-03 15:09 ` [PATCH] mptcp: pm: Fix uaf " Edward Adam Davis
2024-09-03 15:18   ` Eric Dumazet
2024-09-04  0:52     ` Edward Adam Davis
2024-09-04  1:01       ` [PATCH V2] " Edward Adam Davis
2024-09-04 20:39         ` Matthieu Baerts
2024-09-05 12:27           ` Edward Adam Davis [this message]
2024-09-06 18:55             ` [PATCH net v3] " Matthieu Baerts
2024-09-06 22:02               ` Jakub Kicinski
2024-09-09  7:01                 ` Matthieu Baerts
2024-09-09 13:07             ` Paolo Abeni
2024-09-10  3:59               ` Edward Adam Davis
2024-09-10  9:58                 ` [PATCH V4] " Edward Adam Davis
2024-09-10  9:58                 ` [PATCH net " Edward Adam Davis
2024-09-10 11:27                   ` Paolo Abeni
2024-09-10 14:29                   ` Matthieu Baerts
2024-09-11 23:20                   ` patchwork-bot+netdevbpf
2024-09-05 12:35           ` [PATCH V2] " Edward Adam Davis
2024-09-04  0:20 ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " Edward Adam Davis
2024-09-04  0:42   ` syzbot
2024-09-04 17:05 ` [PATCH net] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
2024-09-05  0:46   ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " syzbot
2024-09-04 17:45 ` [PATCH net v2] mptcp: pm: Fix uaf " Matthieu Baerts (NGI0)
2024-09-05  1:10   ` [syzbot] [mptcp?] KASAN: slab-use-after-free Read " syzbot
2024-09-10  3:54 ` Edward Adam Davis
2024-09-10  6:56   ` syzbot
2024-09-10  7:10 ` Edward Adam Davis
2024-09-10  7:43   ` syzbot
2024-09-10  8:03 ` Edward Adam Davis
2024-09-10  8:32   ` syzbot
2024-09-10  9:32 ` Edward Adam Davis
2024-09-10  9:57   ` syzbot

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=tencent_F85DEC5DED99554FB28DEF258F8DB8120D07@qq.com \
    --to=eadavis@qq.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=geliang@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martineau@kernel.org \
    --cc=matttbe@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=syzbot+f3a31fb909db9b2a5c4d@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.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

all inboxes | Powered by JetHome®