[PATCH net v2] tipc: fix use-after-free of the discoverer in tipc_disc_rcv()

[PATCH net v2] tipc: fix use-after-free of the discoverer in tipc_disc_rcv()

[Weiming Shi]

bearer_disable() frees b->disc with tipc_disc_delete()'s plain kfree(),
but tipc_disc_rcv() still dereferences b->disc in RX softirq under
rcu_read_lock() (tipc_udp_recv -> tipc_rcv -> tipc_disc_rcv).

L2 bearers are safe thanks to the synchronize_net() in
tipc_disable_l2_media(), but the UDP bearer defers that call to the
cleanup_bearer() workqueue, so the discoverer is freed with no grace
period:

 BUG: KASAN: slab-use-after-free in tipc_disc_rcv (net/tipc/discover.c:149)
 Read of size 8 at addr ffff88802348b728 by task poc_tipc/184
 <IRQ>
  tipc_disc_rcv (net/tipc/discover.c:149)
  tipc_rcv (net/tipc/node.c:2126)
  tipc_udp_recv (net/tipc/udp_media.c:391)
  udp_rcv (net/ipv4/udp.c:2643)
  ip_local_deliver_finish (net/ipv4/ip_input.c:241)
 </IRQ>
 Freed by task 181:
  kfree (mm/slub.c:6565)
  bearer_disable (net/tipc/bearer.c:418)
  tipc_nl_bearer_disable (net/tipc/bearer.c:1001)

The bearer is freed with kfree_rcu(); free the discoverer the same way.
Add an rcu_head to struct tipc_discoverer and free it and its skb from an
RCU callback.

Because the RCU callback (tipc_disc_free_rcu) lives in module text, a
call_rcu() that is still pending when the tipc module is unloaded would
invoke a freed function. Add an rcu_barrier() to tipc_exit() after the
bearer subsystem has been torn down, so all pending discoverer callbacks
have run before the module text goes away.

Reachable from an unprivileged user namespace: the TIPCv2 genl family is
netnsok and its bearer commands have no GENL_ADMIN_PERM. Needs CONFIG_TIPC
and CONFIG_TIPC_MEDIA_UDP.

Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash values")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v2:
 - split the over-80-column container_of() line (Tung Quang Nguyen)
 - add rcu_barrier() to tipc_exit() so a pending call_rcu() cannot fire
   into freed module text after rmmod (Eric Dumazet)

 net/tipc/core.c     |  3 +++
 net/tipc/discover.c | 14 ++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/net/tipc/core.c b/net/tipc/core.c
index 434e70eabe08..747328e58d30 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -218,6 +218,9 @@ static void __exit tipc_exit(void)
 	unregister_pernet_device(&tipc_net_ops);
 	tipc_unregister_sysctl();
 
+	/* Wait for tipc_disc_free_rcu() callbacks queued from module text. */
+	rcu_barrier();
+
 	pr_info("Deactivated\n");
 }
 
diff --git a/net/tipc/discover.c b/net/tipc/discover.c
index 3e54d2df5683..696b7a8ed54d 100644
--- a/net/tipc/discover.c
+++ b/net/tipc/discover.c
@@ -58,6 +58,7 @@
  * @skb: request message to be (repeatedly) sent
  * @timer: timer governing period between requests
  * @timer_intv: current interval between requests (in ms)
+ * @rcu: RCU head for deferred freeing
  */
 struct tipc_discoverer {
 	u32 bearer_id;
@@ -69,6 +70,7 @@ struct tipc_discoverer {
 	struct sk_buff *skb;
 	struct timer_list timer;
 	unsigned long timer_intv;
+	struct rcu_head rcu;
 };
 
 /**
@@ -382,6 +384,15 @@ int tipc_disc_create(struct net *net, struct tipc_bearer *b,
 	return 0;
 }
 
+static void tipc_disc_free_rcu(struct rcu_head *rp)
+{
+	struct tipc_discoverer *d =
+		container_of(rp, struct tipc_discoverer, rcu);
+
+	kfree_skb(d->skb);
+	kfree(d);
+}
+
 /**
  * tipc_disc_delete - destroy object sending periodic link setup requests
  * @d: ptr to link dest structure
@@ -389,8 +400,7 @@ int tipc_disc_create(struct net *net, struct tipc_bearer *b,
 void tipc_disc_delete(struct tipc_discoverer *d)
 {
 	timer_shutdown_sync(&d->timer);
-	kfree_skb(d->skb);
-	kfree(d);
+	call_rcu(&d->rcu, tipc_disc_free_rcu);
 }
 
 /**
-- 
2.43.0

RE: [PATCH net v2] tipc: fix use-after-free of the discoverer in tipc_disc_rcv()

[Tung Quang Nguyen]

>Subject: [PATCH net v2] tipc: fix use-after-free of the discoverer in
>tipc_disc_rcv()
>
>bearer_disable() frees b->disc with tipc_disc_delete()'s plain kfree(), but
>tipc_disc_rcv() still dereferences b->disc in RX softirq under
>rcu_read_lock() (tipc_udp_recv -> tipc_rcv -> tipc_disc_rcv).
>
>L2 bearers are safe thanks to the synchronize_net() in tipc_disable_l2_media(),
>but the UDP bearer defers that call to the
>cleanup_bearer() workqueue, so the discoverer is freed with no grace
>period:
>
> BUG: KASAN: slab-use-after-free in tipc_disc_rcv (net/tipc/discover.c:149)
>Read of size 8 at addr ffff88802348b728 by task poc_tipc/184  <IRQ>
>  tipc_disc_rcv (net/tipc/discover.c:149)
>  tipc_rcv (net/tipc/node.c:2126)
>  tipc_udp_recv (net/tipc/udp_media.c:391)
>  udp_rcv (net/ipv4/udp.c:2643)
>  ip_local_deliver_finish (net/ipv4/ip_input.c:241)  </IRQ>  Freed by task 181:
>  kfree (mm/slub.c:6565)
>  bearer_disable (net/tipc/bearer.c:418)
>  tipc_nl_bearer_disable (net/tipc/bearer.c:1001)
>
>The bearer is freed with kfree_rcu(); free the discoverer the same way.
>Add an rcu_head to struct tipc_discoverer and free it and its skb from an RCU
>callback.
>
>Because the RCU callback (tipc_disc_free_rcu) lives in module text, a
>call_rcu() that is still pending when the tipc module is unloaded would invoke a
>freed function. Add an rcu_barrier() to tipc_exit() after the bearer subsystem
>has been torn down, so all pending discoverer callbacks have run before the
>module text goes away.
>
>Reachable from an unprivileged user namespace: the TIPCv2 genl family is
>netnsok and its bearer commands have no GENL_ADMIN_PERM. Needs
>CONFIG_TIPC and CONFIG_TIPC_MEDIA_UDP.
>
>Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash
>values")
>Reported-by: Xiang Mei <xmei5@asu.edu>
>Assisted-by: Claude:claude-opus-4-8
>Signed-off-by: Weiming Shi <bestswngs@gmail.com>
>---
>v2:
> - split the over-80-column container_of() line (Tung Quang Nguyen)
> - add rcu_barrier() to tipc_exit() so a pending call_rcu() cannot fire
>   into freed module text after rmmod (Eric Dumazet)
>
> net/tipc/core.c     |  3 +++
> net/tipc/discover.c | 14 ++++++++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
>diff --git a/net/tipc/core.c b/net/tipc/core.c index
>434e70eabe08..747328e58d30 100644
>--- a/net/tipc/core.c
>+++ b/net/tipc/core.c
>@@ -218,6 +218,9 @@ static void __exit tipc_exit(void)
> 	unregister_pernet_device(&tipc_net_ops);
> 	tipc_unregister_sysctl();
>
>+	/* Wait for tipc_disc_free_rcu() callbacks queued from module text. */

Please change above comment to: /* TODO: Wait for all timers that  called call_rcu() to finish before calling rcu_barrier() */

Note that call_rcu() are used in discover.c and node.c. So, the TODO comment helps we add more checking code later in another patch.

>+	rcu_barrier();
>+
> 	pr_info("Deactivated\n");
> }
>
>diff --git a/net/tipc/discover.c b/net/tipc/discover.c index
>3e54d2df5683..696b7a8ed54d 100644
>--- a/net/tipc/discover.c
>+++ b/net/tipc/discover.c
>@@ -58,6 +58,7 @@
>  * @skb: request message to be (repeatedly) sent
>  * @timer: timer governing period between requests
>  * @timer_intv: current interval between requests (in ms)
>+ * @rcu: RCU head for deferred freeing
>  */
> struct tipc_discoverer {
> 	u32 bearer_id;
>@@ -69,6 +70,7 @@ struct tipc_discoverer {
> 	struct sk_buff *skb;
> 	struct timer_list timer;
> 	unsigned long timer_intv;
>+	struct rcu_head rcu;
> };
>
> /**
>@@ -382,6 +384,15 @@ int tipc_disc_create(struct net *net, struct tipc_bearer
>*b,
> 	return 0;
> }
>
>+static void tipc_disc_free_rcu(struct rcu_head *rp) {
>+	struct tipc_discoverer *d =
>+		container_of(rp, struct tipc_discoverer, rcu);
>+
>+	kfree_skb(d->skb);
>+	kfree(d);
>+}
>+
> /**
>  * tipc_disc_delete - destroy object sending periodic link setup requests
>  * @d: ptr to link dest structure
>@@ -389,8 +400,7 @@ int tipc_disc_create(struct net *net, struct tipc_bearer
>*b,  void tipc_disc_delete(struct tipc_discoverer *d)  {
> 	timer_shutdown_sync(&d->timer);
>-	kfree_skb(d->skb);
>-	kfree(d);
>+	call_rcu(&d->rcu, tipc_disc_free_rcu);
> }
>
> /**
>--
>2.43.0

Re: [PATCH net v2] tipc: fix use-after-free of the discoverer in tipc_disc_rcv()

[Weiming Shi]

Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年6月17日周三 16:47写道：
>
> >Subject: [PATCH net v2] tipc: fix use-after-free of the discoverer in
> >tipc_disc_rcv()
> >
> >bearer_disable() frees b->disc with tipc_disc_delete()'s plain kfree(), but
> >tipc_disc_rcv() still dereferences b->disc in RX softirq under
> >rcu_read_lock() (tipc_udp_recv -> tipc_rcv -> tipc_disc_rcv).
> >
> >L2 bearers are safe thanks to the synchronize_net() in tipc_disable_l2_media(),
> >but the UDP bearer defers that call to the
> >cleanup_bearer() workqueue, so the discoverer is freed with no grace
> >period:
> >
> > BUG: KASAN: slab-use-after-free in tipc_disc_rcv (net/tipc/discover.c:149)
> >Read of size 8 at addr ffff88802348b728 by task poc_tipc/184  <IRQ>
> >  tipc_disc_rcv (net/tipc/discover.c:149)
> >  tipc_rcv (net/tipc/node.c:2126)
> >  tipc_udp_recv (net/tipc/udp_media.c:391)
> >  udp_rcv (net/ipv4/udp.c:2643)
> >  ip_local_deliver_finish (net/ipv4/ip_input.c:241)  </IRQ>  Freed by task 181:
> >  kfree (mm/slub.c:6565)
> >  bearer_disable (net/tipc/bearer.c:418)
> >  tipc_nl_bearer_disable (net/tipc/bearer.c:1001)
> >
> >The bearer is freed with kfree_rcu(); free the discoverer the same way.
> >Add an rcu_head to struct tipc_discoverer and free it and its skb from an RCU
> >callback.
> >
> >Because the RCU callback (tipc_disc_free_rcu) lives in module text, a
> >call_rcu() that is still pending when the tipc module is unloaded would invoke a
> >freed function. Add an rcu_barrier() to tipc_exit() after the bearer subsystem
> >has been torn down, so all pending discoverer callbacks have run before the
> >module text goes away.
> >
> >Reachable from an unprivileged user namespace: the TIPCv2 genl family is
> >netnsok and its bearer commands have no GENL_ADMIN_PERM. Needs
> >CONFIG_TIPC and CONFIG_TIPC_MEDIA_UDP.
> >
> >Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash
> >values")
> >Reported-by: Xiang Mei <xmei5@asu.edu>
> >Assisted-by: Claude:claude-opus-4-8
> >Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> >---
> >v2:
> > - split the over-80-column container_of() line (Tung Quang Nguyen)
> > - add rcu_barrier() to tipc_exit() so a pending call_rcu() cannot fire
> >   into freed module text after rmmod (Eric Dumazet)
> >
> > net/tipc/core.c     |  3 +++
> > net/tipc/discover.c | 14 ++++++++++++--
> > 2 files changed, 15 insertions(+), 2 deletions(-)
> >
> >diff --git a/net/tipc/core.c b/net/tipc/core.c index
> >434e70eabe08..747328e58d30 100644
> >--- a/net/tipc/core.c
> >+++ b/net/tipc/core.c
> >@@ -218,6 +218,9 @@ static void __exit tipc_exit(void)
> >       unregister_pernet_device(&tipc_net_ops);
> >       tipc_unregister_sysctl();
> >
> >+      /* Wait for tipc_disc_free_rcu() callbacks queued from module text. */
>
> Please change above comment to: /* TODO: Wait for all timers that  called call_rcu() to finish before calling rcu_barrier() */
>
> Note that call_rcu() are used in discover.c and node.c. So, the TODO comment helps we add more checking code later in another patch.
>
> >+      rcu_barrier();
> >+
> >       pr_info("Deactivated\n");
> > }
> >
> >diff --git a/net/tipc/discover.c b/net/tipc/discover.c index
> >3e54d2df5683..696b7a8ed54d 100644
> >--- a/net/tipc/discover.c
> >+++ b/net/tipc/discover.c
> >@@ -58,6 +58,7 @@
> >  * @skb: request message to be (repeatedly) sent
> >  * @timer: timer governing period between requests
> >  * @timer_intv: current interval between requests (in ms)
> >+ * @rcu: RCU head for deferred freeing
> >  */
> > struct tipc_discoverer {
> >       u32 bearer_id;
> >@@ -69,6 +70,7 @@ struct tipc_discoverer {
> >       struct sk_buff *skb;
> >       struct timer_list timer;
> >       unsigned long timer_intv;
> >+      struct rcu_head rcu;
> > };
> >
> > /**
> >@@ -382,6 +384,15 @@ int tipc_disc_create(struct net *net, struct tipc_bearer
> >*b,
> >       return 0;
> > }
> >
> >+static void tipc_disc_free_rcu(struct rcu_head *rp) {
> >+      struct tipc_discoverer *d =
> >+              container_of(rp, struct tipc_discoverer, rcu);
> >+
> >+      kfree_skb(d->skb);
> >+      kfree(d);
> >+}
> >+
> > /**
> >  * tipc_disc_delete - destroy object sending periodic link setup requests
> >  * @d: ptr to link dest structure
> >@@ -389,8 +400,7 @@ int tipc_disc_create(struct net *net, struct tipc_bearer
> >*b,  void tipc_disc_delete(struct tipc_discoverer *d)  {
> >       timer_shutdown_sync(&d->timer);
> >-      kfree_skb(d->skb);
> >-      kfree(d);
> >+      call_rcu(&d->rcu, tipc_disc_free_rcu);
> > }
> >
> > /**
> >--
> >2.43.0
>

Hi
Sent v3. Thanks for the review.

Best,
Weiming Shi