mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] rtnetlink: wait for RCU readers in rtnl_link_unregister()
@ 2026-09-29 17:04 Andrea Parri
  2026-09-29 18:01 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Andrea Parri @ 2026-09-29 17:04 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Andrea Parri, Simon Horman, Kuniyuki Iwashima, netdev,
	linux-kernel, stable

rtnl_link_ops_get() finds ops on link_ops under rcu_read_lock() and only
then takes ops->srcu. rtnl_link_unregister() unlinks ops, then calls
synchronize_srcu() and cleanup_srcu_struct() without first waiting for
RCU readers. A reader that found ops before list_del_rcu() and has not yet
taken ops->srcu is not waited for.

Such a reader then locks a cleaned-up srcu_struct and keeps using ops
after the module that owns it is gone. rtnl_newlink() and
rtnl_dump_ifinfo() both look up ops without RTNL, the latter for any
user that dumps links filtered by kind.

With delays added between the lookup and srcu_read_lock() in
rtnl_link_ops_get(), and before rtnl_link_ops_put() in
rtnl_dump_ifinfo(), running "ip link show type dummy" alongside
"rmmod dummy" on a KASAN kernel gives:

  BUG: KASAN: vmalloc-out-of-bounds in __srcu_read_unlock+0x57/0x70
  Read of size 8 at addr ffffffffc02ff318 by task ip/147
  Call Trace:
   __srcu_read_unlock+0x57/0x70
   rtnl_dump_ifinfo.cold+0x30/0x3b
   rtnl_dumpit+0x7a/0x1b0
   netlink_dump+0x486/0x11b0
   __netlink_dump_start+0x661/0x910
   rtnetlink_rcv_msg+0x611/0x9e0
   netlink_rcv_skb+0x126/0x380
   netlink_unicast+0x605/0x830
   netlink_sendmsg+0x6d0/0xbb0
   __sys_sendto+0x335/0x3a0

Call synchronize_rcu() before synchronize_srcu(), as
rtnl_af_unregister() already does. This should be enough because
rtnl_link_ops_get() is the only RCU walker of link_ops, and it takes
ops->srcu before rcu_read_unlock(), so every reader that found ops is
covered by synchronize_srcu() once the RCU grace period ends. The extra
grace period only costs time at module unload.

Readers and the writer were serialized by RTNL until link_ops got its
own mutex, which is why the missing RCU wait did not matter before.

Fixes: 6b57ff21a310 ("rtnetlink: Protect link_ops by mutex.")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 net/core/rtnetlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index be9d1625bac31..abcd2c4ea2178 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -707,6 +707,7 @@ void rtnl_link_unregister(struct rtnl_link_ops *ops)
 	list_del_rcu(&ops->list);
 	mutex_unlock(&link_ops_mutex);
 
+	synchronize_rcu();
 	synchronize_srcu(&ops->srcu);
 	cleanup_srcu_struct(&ops->srcu);
 

base-commit: 54518e0e827f4ca9229ae657022c60bf60f5c1bf
-- 
2.53.0


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

* Re: [PATCH net] rtnetlink: wait for RCU readers in rtnl_link_unregister()
  2026-09-29 17:04 [PATCH net] rtnetlink: wait for RCU readers in rtnl_link_unregister() Andrea Parri
@ 2026-09-29 18:01 ` Eric Dumazet
  2026-09-29 22:59   ` Andrea Parri
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2026-09-29 18:01 UTC (permalink / raw)
  To: Andrea Parri, Kuniyuki Iwashima
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	netdev, linux-kernel, stable

On Tue, Sep 29, 2026 at 7:05 PM Andrea Parri <parri.andrea@gmail.com> wrote:
>
> rtnl_link_ops_get() finds ops on link_ops under rcu_read_lock() and only
> then takes ops->srcu. rtnl_link_unregister() unlinks ops, then calls
> synchronize_srcu() and cleanup_srcu_struct() without first waiting for
> RCU readers. A reader that found ops before list_del_rcu() and has not yet
> taken ops->srcu is not waited for.
>
> Such a reader then locks a cleaned-up srcu_struct and keeps using ops
> after the module that owns it is gone. rtnl_newlink() and
> rtnl_dump_ifinfo() both look up ops without RTNL, the latter for any
> user that dumps links filtered by kind.
>
> With delays added between the lookup and srcu_read_lock() in
> rtnl_link_ops_get(), and before rtnl_link_ops_put() in
> rtnl_dump_ifinfo(), running "ip link show type dummy" alongside
> "rmmod dummy" on a KASAN kernel gives:
>
>   BUG: KASAN: vmalloc-out-of-bounds in __srcu_read_unlock+0x57/0x70
>   Read of size 8 at addr ffffffffc02ff318 by task ip/147
>   Call Trace:
>    __srcu_read_unlock+0x57/0x70
>    rtnl_dump_ifinfo.cold+0x30/0x3b
>    rtnl_dumpit+0x7a/0x1b0
>    netlink_dump+0x486/0x11b0
>    __netlink_dump_start+0x661/0x910
>    rtnetlink_rcv_msg+0x611/0x9e0
>    netlink_rcv_skb+0x126/0x380
>    netlink_unicast+0x605/0x830
>    netlink_sendmsg+0x6d0/0xbb0
>    __sys_sendto+0x335/0x3a0
>
> Call synchronize_rcu() before synchronize_srcu(), as
> rtnl_af_unregister() already does. This should be enough because
> rtnl_link_ops_get() is the only RCU walker of link_ops, and it takes
> ops->srcu before rcu_read_unlock(), so every reader that found ops is
> covered by synchronize_srcu() once the RCU grace period ends. The extra
> grace period only costs time at module unload.
>
> Readers and the writer were serialized by RTNL until link_ops got its
> own mutex, which is why the missing RCU wait did not matter before.
>
> Fixes: 6b57ff21a310 ("rtnetlink: Protect link_ops by mutex.")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
>  net/core/rtnetlink.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index be9d1625bac31..abcd2c4ea2178 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -707,6 +707,7 @@ void rtnl_link_unregister(struct rtnl_link_ops *ops)
>         list_del_rcu(&ops->list);
>         mutex_unlock(&link_ops_mutex);
>

Patch looks good, but I would probably add a comment here, because all
these barriers without comments
are a pain.

/* Wait for rtnl_link_ops_get() readers that found ops but have not
yet taken ops->srcu. */
> +       synchronize_rcu();

Adding comments for the other barriers in a future followup would be awesome.

>         synchronize_srcu(&ops->srcu);
>         cleanup_srcu_struct(&ops->srcu);

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

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

* Re: [PATCH net] rtnetlink: wait for RCU readers in rtnl_link_unregister()
  2026-09-29 18:01 ` Eric Dumazet
@ 2026-09-29 22:59   ` Andrea Parri
  0 siblings, 0 replies; 3+ messages in thread
From: Andrea Parri @ 2026-09-29 22:59 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Andrea Parri, Kuniyuki Iwashima, David S. Miller, Jakub Kicinski,
	Paolo Abeni, Simon Horman, netdev, linux-kernel, stable

On Tue, Sep 29, 2026 at 08:01:39PM +0200, Eric Dumazet wrote:
> Patch looks good, but I would probably add a comment here, because all
> these barriers without comments
> are a pain.
>
> /* Wait for rtnl_link_ops_get() readers that found ops but have not
> yet taken ops->srcu. */
> > +       synchronize_rcu();

Will do in v2, thanks.


> Adding comments for the other barriers in a future followup would be awesome.

Sure.  I have a net-next patch ready which comments the remaining waits
in net/core/rtnetlink.c: synchronize_rcu() in rtnl_af_unregister(), the
synchronize_srcu() calls in rtnl_link_unregister() and
rtnl_af_unregister(), and synchronize_net() in rtnl_unregister_all().
I'll send it once this fix reaches net-next, unless you meant a wider
scope than this file.


> Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Thanks for the review,
  Andrea

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

end of thread, other threads:[~2026-09-29 22:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29 17:04 [PATCH net] rtnetlink: wait for RCU readers in rtnl_link_unregister() Andrea Parri
2026-09-29 18:01 ` Eric Dumazet
2026-09-29 22:59   ` Andrea Parri

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®