Netdev List
 help / color / mirror / Atom feed
From: Weiming Shi <bestswngs@gmail.com>
To: Jon Maloy <jmaloy@redhat.com>,
	Tung Nguyen <tung.quang.nguyen@est.tech>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>
Cc: netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, Xiang Mei <xmei5@asu.edu>,
	Weiming Shi <bestswngs@gmail.com>
Subject: [PATCH net v2] tipc: serialize udp bearer replicast list updates
Date: Tue,  7 Jul 2026 09:42:49 -0700	[thread overview]
Message-ID: <20260707164248.821265-2-bestswngs@gmail.com> (raw)

tipc_udp_rcast_add() and cleanup_bearer() both update ub->rcast.list
with list_add_rcu() / list_del_rcu(), but nothing serializes them. The
add runs from the encap receive softirq (via tipc_udp_rcast_disc())
without rtnl_lock(), so it can race the cleanup delete and corrupt the
list:

 list_del corruption. prev->next should be ffff8880298d7ab8,
   but was ffff88802449ad38. (prev=ffff888027e3ec98)
 kernel BUG at lib/list_debug.c:62!
 RIP: __list_del_entry_valid_or_report+0x17a/0x200
 Workqueue: events cleanup_bearer
 Call Trace:
  cleanup_bearer (net/tipc/udp_media.c:811)
  process_one_work (kernel/workqueue.c:3302)
  worker_thread (kernel/workqueue.c:3466)

The bearer can be enabled from an unprivileged user namespace, as the
TIPCv2 generic-netlink ops carry no GENL_ADMIN_PERM.

Add a spinlock to struct udp_bearer and take it around the
list_add_rcu() in tipc_udp_rcast_add() and the list_del_rcu() loop in
cleanup_bearer() so the two writers can no longer corrupt the list.

While here, switch the read-only walk in tipc_udp_is_known_peer() to
list_for_each_entry_rcu(); it never deletes, so list_for_each_entry_safe()
was misleading.

Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
Reported-by: Xiang Mei <xmei5@asu.edu>
Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v2: (per Tung's review)
 - Narrow the lock to just list_add_rcu().
 - Drop the under-lock dup re-check; serializing the writers is enough.
 - Use list_for_each_entry_rcu() in tipc_udp_is_known_peer().

 net/tipc/udp_media.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 62ae7f5b5840..c6aa8c3c54ce 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -94,6 +94,7 @@ struct udp_replicast {
  * @ifindex:	local address scope
  * @work:	used to schedule deferred work on a bearer
  * @rcast:	associated udp_replicast container
+ * @rcast_lock:	serializes updates to @rcast.list
  */
 struct udp_bearer {
 	struct tipc_bearer __rcu *bearer;
@@ -101,6 +102,7 @@ struct udp_bearer {
 	u32 ifindex;
 	struct work_struct work;
 	struct udp_replicast rcast;
+	spinlock_t rcast_lock; /* protects rcast.list */
 };
 
 static int tipc_udp_is_mcast_addr(struct udp_media_addr *addr)
@@ -281,7 +283,7 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
 static bool tipc_udp_is_known_peer(struct tipc_bearer *b,
 				   struct udp_media_addr *addr)
 {
-	struct udp_replicast *rcast, *tmp;
+	struct udp_replicast *rcast;
 	struct udp_bearer *ub;
 
 	ub = rcu_dereference_rtnl(b->media_ptr);
@@ -290,7 +292,7 @@ static bool tipc_udp_is_known_peer(struct tipc_bearer *b,
 		return false;
 	}
 
-	list_for_each_entry_safe(rcast, tmp, &ub->rcast.list, list) {
+	list_for_each_entry_rcu(rcast, &ub->rcast.list, list) {
 		if (!memcmp(&rcast->addr, addr, sizeof(struct udp_media_addr)))
 			return true;
 	}
@@ -326,7 +328,9 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
 		pr_info("New replicast peer: %pI6\n", &rcast->addr.ipv6);
 #endif
 	b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
+	spin_lock_bh(&ub->rcast_lock);
 	list_add_rcu(&rcast->list, &ub->rcast.list);
+	spin_unlock_bh(&ub->rcast_lock);
 	return 0;
 }
 
@@ -679,6 +683,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&ub->rcast.list);
+	spin_lock_init(&ub->rcast_lock);
 
 	if (!attrs[TIPC_NLA_BEARER_UDP_OPTS])
 		goto err;
@@ -819,10 +824,12 @@ static void cleanup_bearer(struct work_struct *work)
 	struct udp_replicast *rcast, *tmp;
 	struct tipc_net *tn;
 
+	spin_lock_bh(&ub->rcast_lock);
 	list_for_each_entry_safe(rcast, tmp, &ub->rcast.list, list) {
 		list_del_rcu(&rcast->list);
 		call_rcu_hurry(&rcast->rcu, rcast_free_rcu);
 	}
+	spin_unlock_bh(&ub->rcast_lock);
 
 	tn = tipc_net(sock_net(ub->sk));
 
-- 
2.43.0


                 reply	other threads:[~2026-07-07 16:43 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20260707164248.821265-2-bestswngs@gmail.com \
    --to=bestswngs@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jmaloy@redhat.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=tipc-discussion@lists.sourceforge.net \
    --cc=tung.quang.nguyen@est.tech \
    --cc=xmei5@asu.edu \
    /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