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

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

[Weiming Shi]

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 <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 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(). */
+	spin_lock_bh(&ub->rcast_lock);
+	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;
+		}
+	}
+
 	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

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

[Tung Quang Nguyen]

>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 <xmei5@asu.edu>
>Assisted-by: Claude:claude-opus-4-8
>Signed-off-by: Weiming Shi <bestswngs@gmail.com>
>---
> 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().

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

>+	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;
        }

>+
> 	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
>

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

[Weiming Shi]

Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年7月7日周二 21:21写道：
>
> >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 <xmei5@asu.edu>
> >Assisted-by: Claude:claude-opus-4-8
> >Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> >---
> > 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().
>
> >+      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.
>
> >+      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;
>         }
>
> >+
> >       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.