Thread (3 messages) 3 messages, 2 authors, 1d ago

Re: [PATCH net] tipc: serialize udp bearer replicast list updates

From: Weiming Shi <hidden>
Date: 2026-07-07 16:35:26

Tung Quang Nguyen [off-list ref] 于2026年7月7日周二 21:21写道:
quoted hunk ↗ jump to hunk
quoted
Subject: [PATCH net] tipc: serialize udp bearer replicast list updates

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 and take it around both list updates. Re-check for the peer
under the lock in the add path so the check-then-add in
tipc_udp_rcast_disc() cannot insert a duplicate.

Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
Reported-by: Xiang Mei <redacted>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <redacted>
---
net/tipc/udp_media.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c index
62ae7f5b5840..601174297a16 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) @@ -301,7
+303,7 @@ static bool tipc_udp_is_known_peer(struct tipc_bearer *b,  static
int tipc_udp_rcast_add(struct tipc_bearer *b,
                            struct udp_media_addr *addr)
{
-      struct udp_replicast *rcast;
+      struct udp_replicast *rcast, *tmp;
      struct udp_bearer *ub;

      ub = rcu_dereference_rtnl(b->media_ptr);
@@ -319,6 +321,17 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,

      memcpy(&rcast->addr, addr, sizeof(struct udp_media_addr));

+      /* tipc_udp_rcast_disc() adds from softirq without rtnl_lock(). */
Remove above comment because it is vague. Caller of tipc_udp_rcv() already holds rcu_read_lock().
quoted
+      spin_lock_bh(&ub->rcast_lock);
Move 'spin_lock_bh(&ub->rcast_lock)' to above 'list_add_rcu(&rcast->list, &ub->rcast.list);' because only 'ub->rcast.list' needs protection.
quoted
+      list_for_each_entry(tmp, &ub->rcast.list, list) {
+              if (!memcmp(&tmp->addr, addr, sizeof(struct
udp_media_addr))) {
+                      spin_unlock_bh(&ub->rcast_lock);
+                      dst_cache_destroy(&rcast->dst_cache);
+                      kfree(rcast);
+                      return 0;
+              }
+      }
This is wrong because the code in the loop is never executed. tipc_udp_is_known_peer() already verifies address duplication.
Please remove above code and fix tipc_udp_is_known_peer() as below:

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);
@@ -292,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;
        }
quoted
+
      if (ntohs(addr->proto) == ETH_P_IP)
              pr_info("New replicast peer: %pI4\n", &rcast->addr.ipv4);  #if
IS_ENABLED(CONFIG_IPV6) @@ -327,6 +340,7 @@ static int
tipc_udp_rcast_add(struct tipc_bearer *b,  #endif
      b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
      list_add_rcu(&rcast->list, &ub->rcast.list);
+      spin_unlock_bh(&ub->rcast_lock);
      return 0;
}
@@ -679,6 +693,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 +834,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
Thank you for the review. I will look into your suggestions and send out v2.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help