mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v4] tipc: serialize udp bearer replicast list updates
@ 2026-07-10 17:47 Weiming Shi
  2026-07-13 12:06 ` Tung Quang Nguyen
  0 siblings, 1 reply; 3+ messages in thread
From: Weiming Shi @ 2026-07-10 17:47 UTC (permalink / raw)
  To: Jon Maloy, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: netdev, tipc-discussion, linux-kernel, Tung Nguyen, xmei5, 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 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.

Reject a duplicate peer under the same lock before allocating, and remove
tipc_udp_is_known_peer(). The old lockless pre-check in
tipc_udp_rcast_disc() was racy: two softirqs discovering the same peer
could both find it absent and add it twice. Doing the check under
rcast_lock in tipc_udp_rcast_add(), before the allocation, makes it the
single point of truth for both the discovery and the netlink add paths and
keeps a flood of the same address from churning the allocator.

Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
Reported-by: Xiang Mei <xmei5@asu.edu>
Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v4: (per Tung's review)
 - Reject a duplicate under rcast_lock before allocating rcast, so a
   flood of the same address can't churn the allocator.
v3:
 - Check for a duplicate peer in tipc_udp_rcast_add() under rcast_lock
   and remove tipc_udp_is_known_peer().
v2:
 - Narrow the lock to the list mutation.

 net/tipc/udp_media.c | 53 ++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 62ae7f5b5..3f0d75d72 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:	serialize updates to @rcast.list against concurrent updaters
  */
 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)
@@ -278,26 +280,6 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
 	return err;
 }
 
-static bool tipc_udp_is_known_peer(struct tipc_bearer *b,
-				   struct udp_media_addr *addr)
-{
-	struct udp_replicast *rcast, *tmp;
-	struct udp_bearer *ub;
-
-	ub = rcu_dereference_rtnl(b->media_ptr);
-	if (!ub) {
-		pr_err_ratelimited("UDP bearer instance not found\n");
-		return false;
-	}
-
-	list_for_each_entry_safe(rcast, tmp, &ub->rcast.list, list) {
-		if (!memcmp(&rcast->addr, addr, sizeof(struct udp_media_addr)))
-			return true;
-	}
-
-	return false;
-}
-
 static int tipc_udp_rcast_add(struct tipc_bearer *b,
 			      struct udp_media_addr *addr)
 {
@@ -308,16 +290,34 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
 	if (!ub)
 		return -ENODEV;
 
+	/* Serialize with other updaters and reject a duplicate before
+	 * allocating, so a flood of the same address can't churn the
+	 * allocator.
+	 */
+	spin_lock_bh(&ub->rcast_lock);
+	list_for_each_entry(rcast, &ub->rcast.list, list) {
+		if (!memcmp(&rcast->addr, addr, sizeof(*addr))) {
+			spin_unlock_bh(&ub->rcast_lock);
+			return 0;
+		}
+	}
+
 	rcast = kmalloc_obj(*rcast, GFP_ATOMIC);
-	if (!rcast)
+	if (!rcast) {
+		spin_unlock_bh(&ub->rcast_lock);
 		return -ENOMEM;
+	}
 
 	if (dst_cache_init(&rcast->dst_cache, GFP_ATOMIC)) {
+		spin_unlock_bh(&ub->rcast_lock);
 		kfree(rcast);
 		return -ENOMEM;
 	}
 
 	memcpy(&rcast->addr, addr, sizeof(struct udp_media_addr));
+	list_add_rcu(&rcast->list, &ub->rcast.list);
+	b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
+	spin_unlock_bh(&ub->rcast_lock);
 
 	if (ntohs(addr->proto) == ETH_P_IP)
 		pr_info("New replicast peer: %pI4\n", &rcast->addr.ipv4);
@@ -325,8 +325,6 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
 	else if (ntohs(addr->proto) == ETH_P_IPV6)
 		pr_info("New replicast peer: %pI6\n", &rcast->addr.ipv6);
 #endif
-	b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
-	list_add_rcu(&rcast->list, &ub->rcast.list);
 	return 0;
 }
 
@@ -361,9 +359,6 @@ static int tipc_udp_rcast_disc(struct tipc_bearer *b, struct sk_buff *skb)
 		return 0;
 	}
 
-	if (likely(tipc_udp_is_known_peer(b, &src)))
-		return 0;
-
 	return tipc_udp_rcast_add(b, &src);
 }
 
@@ -644,9 +639,6 @@ int tipc_udp_nl_bearer_add(struct tipc_bearer *b, struct nlattr *attr)
 		return -EINVAL;
 	}
 
-	if (tipc_udp_is_known_peer(b, &addr))
-		return 0;
-
 	return tipc_udp_rcast_add(b, &addr);
 }
 
@@ -679,6 +671,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 +812,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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH net v4] tipc: serialize udp bearer replicast list updates
  2026-07-10 17:47 [PATCH net v4] tipc: serialize udp bearer replicast list updates Weiming Shi
@ 2026-07-13 12:06 ` Tung Quang Nguyen
  2026-07-14 19:22   ` Weiming Shi
  0 siblings, 1 reply; 3+ messages in thread
From: Tung Quang Nguyen @ 2026-07-13 12:06 UTC (permalink / raw)
  To: Weiming Shi
  Cc: netdev, tipc-discussion, linux-kernel, xmei5, Jon Maloy,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman

>Subject: [PATCH net v4] 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 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.
>
>Reject a duplicate peer under the same lock before allocating, and remove
>tipc_udp_is_known_peer(). The old lockless pre-check in
>tipc_udp_rcast_disc() was racy: two softirqs discovering the same peer could
>both find it absent and add it twice. Doing the check under rcast_lock in
>tipc_udp_rcast_add(), before the allocation, makes it the single point of truth
>for both the discovery and the netlink add paths and keeps a flood of the same
>address from churning the allocator.
>
>Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
>Reported-by: Xiang Mei <xmei5@asu.edu>
>Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
>Signed-off-by: Weiming Shi <bestswngs@gmail.com>
>---
>v4: (per Tung's review)
> - Reject a duplicate under rcast_lock before allocating rcast, so a
>   flood of the same address can't churn the allocator.
>v3:
> - Check for a duplicate peer in tipc_udp_rcast_add() under rcast_lock
>   and remove tipc_udp_is_known_peer().
>v2:
> - Narrow the lock to the list mutation.
>
> net/tipc/udp_media.c | 53 ++++++++++++++++++++------------------------
> 1 file changed, 24 insertions(+), 29 deletions(-)
>
>diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c index
>62ae7f5b5..3f0d75d72 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:	serialize updates to @rcast.list against concurrent
>updaters
>  */
> 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) @@ -278,26
>+280,6 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
> 	return err;
> }
>
>-static bool tipc_udp_is_known_peer(struct tipc_bearer *b,
>-				   struct udp_media_addr *addr)
>-{
>-	struct udp_replicast *rcast, *tmp;
>-	struct udp_bearer *ub;
>-
>-	ub = rcu_dereference_rtnl(b->media_ptr);
>-	if (!ub) {
>-		pr_err_ratelimited("UDP bearer instance not found\n");
>-		return false;
>-	}
>-
>-	list_for_each_entry_safe(rcast, tmp, &ub->rcast.list, list) {
>-		if (!memcmp(&rcast->addr, addr, sizeof(struct
>udp_media_addr)))
>-			return true;
>-	}
>-
>-	return false;
>-}
>-
> static int tipc_udp_rcast_add(struct tipc_bearer *b,
> 			      struct udp_media_addr *addr)
> {
>@@ -308,16 +290,34 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
> 	if (!ub)
> 		return -ENODEV;
>
>+	/* Serialize with other updaters and reject a duplicate before
>+	 * allocating, so a flood of the same address can't churn the
>+	 * allocator.
>+	 */
>+	spin_lock_bh(&ub->rcast_lock);
>+	list_for_each_entry(rcast, &ub->rcast.list, list) {
>+		if (!memcmp(&rcast->addr, addr, sizeof(*addr))) {
>+			spin_unlock_bh(&ub->rcast_lock);
>+			return 0;
>+		}
>+	}
>+
> 	rcast = kmalloc_obj(*rcast, GFP_ATOMIC);
>-	if (!rcast)
>+	if (!rcast) {
>+		spin_unlock_bh(&ub->rcast_lock);
> 		return -ENOMEM;
>+	}
>
> 	if (dst_cache_init(&rcast->dst_cache, GFP_ATOMIC)) {
>+		spin_unlock_bh(&ub->rcast_lock);
> 		kfree(rcast);
> 		return -ENOMEM;
> 	}
>
> 	memcpy(&rcast->addr, addr, sizeof(struct udp_media_addr));
>+	list_add_rcu(&rcast->list, &ub->rcast.list);
>+	b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
>+	spin_unlock_bh(&ub->rcast_lock);

sashiko reports potential memory leak when tipc_udp_nl_bearer_add() adds an element to the list right after cleanup_bearer() frees all element in the list.
I think we need to add more code to handle this case as below:

--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -95,6 +95,7 @@ struct udp_replicast {
  * @work:      used to schedule deferred work on a bearer
  * @rcast:     associated udp_replicast container
  * @rcast_lock:        serialize updates to @rcast.list against concurrent updaters
+ * @bearer_disabled: flag to not add rcast to rcast.list if bearer was disabled
  */
 struct udp_bearer {
        struct tipc_bearer __rcu *bearer;
@@ -103,6 +104,7 @@ struct udp_bearer {
        struct work_struct work;
        struct udp_replicast rcast;
        spinlock_t rcast_lock; /* protects rcast.list */
+       bool bearer_disabled;
 };
 
 static int tipc_udp_is_mcast_addr(struct udp_media_addr *addr)
@@ -295,6 +297,10 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
         * allocator.
         */
        spin_lock_bh(&ub->rcast_lock);
+       if (ub->bearer_disabled) {
+               spin_unlock_bh(&ub->rcast_lock);
+               return 0;
+       }
        list_for_each_entry(rcast, &ub->rcast.list, list) {
                if (!memcmp(&rcast->addr, addr, sizeof(*addr))) {
                        spin_unlock_bh(&ub->rcast_lock);
@@ -817,6 +823,7 @@ static void cleanup_bearer(struct work_struct *work)
                list_del_rcu(&rcast->list);
                call_rcu_hurry(&rcast->rcu, rcast_free_rcu);
        }
+       ub->bearer_disabled = true;
        spin_unlock_bh(&ub->rcast_lock);
 
        tn = tipc_net(sock_net(ub->sk));

>
> 	if (ntohs(addr->proto) == ETH_P_IP)
> 		pr_info("New replicast peer: %pI4\n", &rcast->addr.ipv4); @@
>-325,8 +325,6 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
> 	else if (ntohs(addr->proto) == ETH_P_IPV6)
> 		pr_info("New replicast peer: %pI6\n", &rcast->addr.ipv6);
>#endif
>-	b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
>-	list_add_rcu(&rcast->list, &ub->rcast.list);
> 	return 0;
> }
>
>@@ -361,9 +359,6 @@ static int tipc_udp_rcast_disc(struct tipc_bearer *b,
>struct sk_buff *skb)
> 		return 0;
> 	}
>
>-	if (likely(tipc_udp_is_known_peer(b, &src)))
>-		return 0;
>-
> 	return tipc_udp_rcast_add(b, &src);
> }
>
>@@ -644,9 +639,6 @@ int tipc_udp_nl_bearer_add(struct tipc_bearer *b,
>struct nlattr *attr)
> 		return -EINVAL;
> 	}
>
>-	if (tipc_udp_is_known_peer(b, &addr))
>-		return 0;
>-
> 	return tipc_udp_rcast_add(b, &addr);
> }
>
>@@ -679,6 +671,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 +812,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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v4] tipc: serialize udp bearer replicast list updates
  2026-07-13 12:06 ` Tung Quang Nguyen
@ 2026-07-14 19:22   ` Weiming Shi
  0 siblings, 0 replies; 3+ messages in thread
From: Weiming Shi @ 2026-07-14 19:22 UTC (permalink / raw)
  To: Tung Quang Nguyen
  Cc: netdev, tipc-discussion, linux-kernel, xmei5, Jon Maloy,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman

Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年7月13日周一 20:06写道:
>
> >Subject: [PATCH net v4] 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 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.
> >
> >Reject a duplicate peer under the same lock before allocating, and remove
> >tipc_udp_is_known_peer(). The old lockless pre-check in
> >tipc_udp_rcast_disc() was racy: two softirqs discovering the same peer could
> >both find it absent and add it twice. Doing the check under rcast_lock in
> >tipc_udp_rcast_add(), before the allocation, makes it the single point of truth
> >for both the discovery and the netlink add paths and keeps a flood of the same
> >address from churning the allocator.
> >
> >Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
> >Reported-by: Xiang Mei <xmei5@asu.edu>
> >Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
> >Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> >---
> >v4: (per Tung's review)
> > - Reject a duplicate under rcast_lock before allocating rcast, so a
> >   flood of the same address can't churn the allocator.
> >v3:
> > - Check for a duplicate peer in tipc_udp_rcast_add() under rcast_lock
> >   and remove tipc_udp_is_known_peer().
> >v2:
> > - Narrow the lock to the list mutation.
> >
> > net/tipc/udp_media.c | 53 ++++++++++++++++++++------------------------
> > 1 file changed, 24 insertions(+), 29 deletions(-)
> >
> >diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c index
> >62ae7f5b5..3f0d75d72 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:       serialize updates to @rcast.list against concurrent
> >updaters
> >  */
> > 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) @@ -278,26
> >+280,6 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
> >       return err;
> > }
> >
> >-static bool tipc_udp_is_known_peer(struct tipc_bearer *b,
> >-                                 struct udp_media_addr *addr)
> >-{
> >-      struct udp_replicast *rcast, *tmp;
> >-      struct udp_bearer *ub;
> >-
> >-      ub = rcu_dereference_rtnl(b->media_ptr);
> >-      if (!ub) {
> >-              pr_err_ratelimited("UDP bearer instance not found\n");
> >-              return false;
> >-      }
> >-
> >-      list_for_each_entry_safe(rcast, tmp, &ub->rcast.list, list) {
> >-              if (!memcmp(&rcast->addr, addr, sizeof(struct
> >udp_media_addr)))
> >-                      return true;
> >-      }
> >-
> >-      return false;
> >-}
> >-
> > static int tipc_udp_rcast_add(struct tipc_bearer *b,
> >                             struct udp_media_addr *addr)
> > {
> >@@ -308,16 +290,34 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
> >       if (!ub)
> >               return -ENODEV;
> >
> >+      /* Serialize with other updaters and reject a duplicate before
> >+       * allocating, so a flood of the same address can't churn the
> >+       * allocator.
> >+       */
> >+      spin_lock_bh(&ub->rcast_lock);
> >+      list_for_each_entry(rcast, &ub->rcast.list, list) {
> >+              if (!memcmp(&rcast->addr, addr, sizeof(*addr))) {
> >+                      spin_unlock_bh(&ub->rcast_lock);
> >+                      return 0;
> >+              }
> >+      }
> >+
> >       rcast = kmalloc_obj(*rcast, GFP_ATOMIC);
> >-      if (!rcast)
> >+      if (!rcast) {
> >+              spin_unlock_bh(&ub->rcast_lock);
> >               return -ENOMEM;
> >+      }
> >
> >       if (dst_cache_init(&rcast->dst_cache, GFP_ATOMIC)) {
> >+              spin_unlock_bh(&ub->rcast_lock);
> >               kfree(rcast);
> >               return -ENOMEM;
> >       }
> >
> >       memcpy(&rcast->addr, addr, sizeof(struct udp_media_addr));
> >+      list_add_rcu(&rcast->list, &ub->rcast.list);
> >+      b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
> >+      spin_unlock_bh(&ub->rcast_lock);
>
> sashiko reports potential memory leak when tipc_udp_nl_bearer_add() adds an element to the list right after cleanup_bearer() frees all element in the list.
> I think we need to add more code to handle this case as below:
>
> --- a/net/tipc/udp_media.c
> +++ b/net/tipc/udp_media.c
> @@ -95,6 +95,7 @@ struct udp_replicast {
>   * @work:      used to schedule deferred work on a bearer
>   * @rcast:     associated udp_replicast container
>   * @rcast_lock:        serialize updates to @rcast.list against concurrent updaters
> + * @bearer_disabled: flag to not add rcast to rcast.list if bearer was disabled
>   */
>  struct udp_bearer {
>         struct tipc_bearer __rcu *bearer;
> @@ -103,6 +104,7 @@ struct udp_bearer {
>         struct work_struct work;
>         struct udp_replicast rcast;
>         spinlock_t rcast_lock; /* protects rcast.list */
> +       bool bearer_disabled;
>  };
>
>  static int tipc_udp_is_mcast_addr(struct udp_media_addr *addr)
> @@ -295,6 +297,10 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
>          * allocator.
>          */
>         spin_lock_bh(&ub->rcast_lock);
> +       if (ub->bearer_disabled) {
> +               spin_unlock_bh(&ub->rcast_lock);
> +               return 0;
> +       }
>         list_for_each_entry(rcast, &ub->rcast.list, list) {
>                 if (!memcmp(&rcast->addr, addr, sizeof(*addr))) {
>                         spin_unlock_bh(&ub->rcast_lock);
> @@ -817,6 +823,7 @@ static void cleanup_bearer(struct work_struct *work)
>                 list_del_rcu(&rcast->list);
>                 call_rcu_hurry(&rcast->rcu, rcast_free_rcu);
>         }
> +       ub->bearer_disabled = true;
>         spin_unlock_bh(&ub->rcast_lock);
>
>         tn = tipc_net(sock_net(ub->sk));
>
> >
> >       if (ntohs(addr->proto) == ETH_P_IP)
> >               pr_info("New replicast peer: %pI4\n", &rcast->addr.ipv4); @@
> >-325,8 +325,6 @@ static int tipc_udp_rcast_add(struct tipc_bearer *b,
> >       else if (ntohs(addr->proto) == ETH_P_IPV6)
> >               pr_info("New replicast peer: %pI6\n", &rcast->addr.ipv6);
> >#endif
> >-      b->bcast_addr.broadcast = TIPC_REPLICAST_SUPPORT;
> >-      list_add_rcu(&rcast->list, &ub->rcast.list);
> >       return 0;
> > }
> >
> >@@ -361,9 +359,6 @@ static int tipc_udp_rcast_disc(struct tipc_bearer *b,
> >struct sk_buff *skb)
> >               return 0;
> >       }
> >
> >-      if (likely(tipc_udp_is_known_peer(b, &src)))
> >-              return 0;
> >-
> >       return tipc_udp_rcast_add(b, &src);
> > }
> >
> >@@ -644,9 +639,6 @@ int tipc_udp_nl_bearer_add(struct tipc_bearer *b,
> >struct nlattr *attr)
> >               return -EINVAL;
> >       }
> >
> >-      if (tipc_udp_is_known_peer(b, &addr))
> >-              return 0;
> >-
> >       return tipc_udp_rcast_add(b, &addr);
> > }
> >
> >@@ -679,6 +671,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 +812,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
>

v5 sent. Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-14 19:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-10 17:47 [PATCH net v4] tipc: serialize udp bearer replicast list updates Weiming Shi
2026-07-13 12:06 ` Tung Quang Nguyen
2026-07-14 19:22   ` Weiming Shi

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®