Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink()
@ 2026-06-02  9:13 Eric Dumazet
  2026-06-02 12:23 ` Jiayuan Chen
  2026-06-02 17:57 ` Jakub Kicinski
  0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2026-06-02  9:13 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Kuniyuki Iwashima, Simon Horman, netdev, eric.dumazet,
	Eric Dumazet

rtnl_getlink() uses an RCU lookup to get the netdevice pointer.

When/If rtnl_lock() is used, we should check if the netdevice is not
being dismantled before potentially perform illegal actions.

Move dev_isalive() out of net/core/net-sysfs.c and make it available
in include/linux/netdevice.h

Return -ENODEV if rtnl_getlink() finds a device which is currently
being dismantled and RTNL is requested.

Fixes: e896e5c0734b ("rtnetlink: do not acquire RTNL in rtnl_getlink() with RTEXT_FILTER_NAME_ONLY")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
---
 include/linux/netdevice.h | 6 ++++++
 net/core/net-sysfs.c      | 6 ------
 net/core/rtnetlink.c      | 5 +++++
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 74507c006490f180d2fac6594f6dcf2c86d53919..bfc847bc47fd957b58174dc4d2b82e2e8b461405 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5660,6 +5660,12 @@ static inline const char *netdev_name(const struct net_device *dev)
 	return dev->name;
 }
 
+/* Caller holds RTNL, netdev->lock or RCU */
+static inline bool dev_isalive(const struct net_device *dev)
+{
+	return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
+}
+
 static inline const char *netdev_reg_state(const struct net_device *dev)
 {
 	u8 reg_state = READ_ONCE(dev->reg_state);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 3318b5666e438fe5bab2bcfed2bb260c7b4d5a63..0e71c9ed41e81d85af33a4339f556a0c5d760243 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -37,12 +37,6 @@ static const char fmt_uint[] = "%u\n";
 static const char fmt_ulong[] = "%lu\n";
 static const char fmt_u64[] = "%llu\n";
 
-/* Caller holds RTNL, netdev->lock or RCU */
-static inline int dev_isalive(const struct net_device *dev)
-{
-	return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
-}
-
 /* There is a possible ABBA deadlock between rtnl_lock and kernfs_node->active,
  * when unregistering a net device and accessing associated sysfs files. The
  * potential deadlock is as follow:
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 652dd008955a90691403de9a54d8693d64ea7799..9e90d02b73c2909409e12adfcec3940b49dcbaa1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4265,6 +4265,10 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 retry:
 	if (need_rtnl) {
 		rtnl_lock();
+		if (!dev_isalive(dev)) {
+			err = -ENODEV;
+			goto unlock;
+		}
 		/* Synchronize the carrier state so we don't report a state
 		 * that we're not actually going to honour immediately; if
 		 * the driver just did a carrier off->on transition, we can
@@ -4282,6 +4286,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 				       nlh->nlmsg_seq, 0, 0, ext_filter_mask,
 				       0, NULL, 0, netnsid, GFP_KERNEL);
 
+unlock:
 	if (need_rtnl)
 		rtnl_unlock();
 
-- 
2.54.0.929.g9b7fa37559-goog


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

* Re: [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink()
  2026-06-02  9:13 [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink() Eric Dumazet
@ 2026-06-02 12:23 ` Jiayuan Chen
  2026-06-02 12:47   ` Eric Dumazet
  2026-06-02 17:57 ` Jakub Kicinski
  1 sibling, 1 reply; 5+ messages in thread
From: Jiayuan Chen @ 2026-06-02 12:23 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Kuniyuki Iwashima, Simon Horman, netdev, eric.dumazet


On 6/2/26 5:13 PM, Eric Dumazet wrote:
> rtnl_getlink() uses an RCU lookup to get the netdevice pointer.
>
> When/If rtnl_lock() is used, we should check if the netdevice is not
> being dismantled before potentially perform illegal actions.
>
> Move dev_isalive() out of net/core/net-sysfs.c and make it available
> in include/linux/netdevice.h
>
> Return -ENODEV if rtnl_getlink() finds a device which is currently
> being dismantled and RTNL is requested.
>
> Fixes: e896e5c0734b ("rtnetlink: do not acquire RTNL in rtnl_getlink() with RTEXT_FILTER_NAME_ONLY")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> ---
>   include/linux/netdevice.h | 6 ++++++
>   net/core/net-sysfs.c      | 6 ------
>   net/core/rtnetlink.c      | 5 +++++
>   3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 74507c006490f180d2fac6594f6dcf2c86d53919..bfc847bc47fd957b58174dc4d2b82e2e8b461405 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -5660,6 +5660,12 @@ static inline const char *netdev_name(const struct net_device *dev)
>   	return dev->name;
>   }
>   
> +/* Caller holds RTNL, netdev->lock or RCU */
> +static inline bool dev_isalive(const struct net_device *dev)
> +{
> +	return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
> +}
> +
>   static inline const char *netdev_reg_state(const struct net_device *dev)
>   {
>   	u8 reg_state = READ_ONCE(dev->reg_state);
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 3318b5666e438fe5bab2bcfed2bb260c7b4d5a63..0e71c9ed41e81d85af33a4339f556a0c5d760243 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -37,12 +37,6 @@ static const char fmt_uint[] = "%u\n";
>   static const char fmt_ulong[] = "%lu\n";
>   static const char fmt_u64[] = "%llu\n";
>   
> -/* Caller holds RTNL, netdev->lock or RCU */
> -static inline int dev_isalive(const struct net_device *dev)
> -{
> -	return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
> -}
> -
>   /* There is a possible ABBA deadlock between rtnl_lock and kernfs_node->active,
>    * when unregistering a net device and accessing associated sysfs files. The
>    * potential deadlock is as follow:
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 652dd008955a90691403de9a54d8693d64ea7799..9e90d02b73c2909409e12adfcec3940b49dcbaa1 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4265,6 +4265,10 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>   retry:
>   	if (need_rtnl) {
>   		rtnl_lock();
> +		if (!dev_isalive(dev)) {
> +			err = -ENODEV;
> +			goto unlock;


This does fix the macvlan UAF, thanks.

But 'nskb' is used uninitialized, so the kfree_skb(nskb) below frees an 
uninitialized pointer.

+unlock:
     if (need_rtnl)
             rtnl_unlock();

     if (err < 0) {
             kfree_skb(nskb);   /* nskb is uninitialized when reached 
via goto unlock */
             ...
     }


> +		}
>   		/* Synchronize the carrier state so we don't report a state
>   		 * that we're not actually going to honour immediately; if
>   		 * the driver just did a carrier off->on transition, we can
> @@ -4282,6 +4286,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>   				       nlh->nlmsg_seq, 0, 0, ext_filter_mask,
>   				       0, NULL, 0, netnsid, GFP_KERNEL);
>   
> +unlock:
>   	if (need_rtnl)
>   		rtnl_unlock();
>   




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

* Re: [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink()
  2026-06-02 12:23 ` Jiayuan Chen
@ 2026-06-02 12:47   ` Eric Dumazet
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2026-06-02 12:47 UTC (permalink / raw)
  To: Jiayuan Chen
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
	Simon Horman, netdev, eric.dumazet

On Tue, Jun 2, 2026 at 5:23 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>
> On 6/2/26 5:13 PM, Eric Dumazet wrote:
> > rtnl_getlink() uses an RCU lookup to get the netdevice pointer.
> >
> > When/If rtnl_lock() is used, we should check if the netdevice is not
> > being dismantled before potentially perform illegal actions.
> >
> > Move dev_isalive() out of net/core/net-sysfs.c and make it available
> > in include/linux/netdevice.h
> >
> > Return -ENODEV if rtnl_getlink() finds a device which is currently
> > being dismantled and RTNL is requested.
> >
> > Fixes: e896e5c0734b ("rtnetlink: do not acquire RTNL in rtnl_getlink() with RTEXT_FILTER_NAME_ONLY")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >   include/linux/netdevice.h | 6 ++++++
> >   net/core/net-sysfs.c      | 6 ------
> >   net/core/rtnetlink.c      | 5 +++++
> >   3 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 74507c006490f180d2fac6594f6dcf2c86d53919..bfc847bc47fd957b58174dc4d2b82e2e8b461405 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -5660,6 +5660,12 @@ static inline const char *netdev_name(const struct net_device *dev)
> >       return dev->name;
> >   }
> >
> > +/* Caller holds RTNL, netdev->lock or RCU */
> > +static inline bool dev_isalive(const struct net_device *dev)
> > +{
> > +     return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
> > +}
> > +
> >   static inline const char *netdev_reg_state(const struct net_device *dev)
> >   {
> >       u8 reg_state = READ_ONCE(dev->reg_state);
> > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> > index 3318b5666e438fe5bab2bcfed2bb260c7b4d5a63..0e71c9ed41e81d85af33a4339f556a0c5d760243 100644
> > --- a/net/core/net-sysfs.c
> > +++ b/net/core/net-sysfs.c
> > @@ -37,12 +37,6 @@ static const char fmt_uint[] = "%u\n";
> >   static const char fmt_ulong[] = "%lu\n";
> >   static const char fmt_u64[] = "%llu\n";
> >
> > -/* Caller holds RTNL, netdev->lock or RCU */
> > -static inline int dev_isalive(const struct net_device *dev)
> > -{
> > -     return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
> > -}
> > -
> >   /* There is a possible ABBA deadlock between rtnl_lock and kernfs_node->active,
> >    * when unregistering a net device and accessing associated sysfs files. The
> >    * potential deadlock is as follow:
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index 652dd008955a90691403de9a54d8693d64ea7799..9e90d02b73c2909409e12adfcec3940b49dcbaa1 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -4265,6 +4265,10 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> >   retry:
> >       if (need_rtnl) {
> >               rtnl_lock();
> > +             if (!dev_isalive(dev)) {
> > +                     err = -ENODEV;
> > +                     goto unlock;
>
>
> This does fix the macvlan UAF, thanks.
>
> But 'nskb' is used uninitialized, so the kfree_skb(nskb) below frees an
> uninitialized pointer.
>
> +unlock:
>      if (need_rtnl)
>              rtnl_unlock();
>
>      if (err < 0) {
>              kfree_skb(nskb);   /* nskb is uninitialized when reached
> via goto unlock */
>              ...
>      }

Oh right, last minute refactoring change, I will send a V2,

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

* Re: [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink()
  2026-06-02  9:13 [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink() Eric Dumazet
  2026-06-02 12:23 ` Jiayuan Chen
@ 2026-06-02 17:57 ` Jakub Kicinski
  2026-06-02 18:36   ` Eric Dumazet
  1 sibling, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2026-06-02 17:57 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Paolo Abeni, Kuniyuki Iwashima, Simon Horman,
	netdev, eric.dumazet

On Tue,  2 Jun 2026 09:13:19 +0000 Eric Dumazet wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 74507c006490f180d2fac6594f6dcf2c86d53919..bfc847bc47fd957b58174dc4d2b82e2e8b461405 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -5660,6 +5660,12 @@ static inline const char *netdev_name(const struct net_device *dev)
>  	return dev->name;
>  }
>  
> +/* Caller holds RTNL, netdev->lock or RCU */
> +static inline bool dev_isalive(const struct net_device *dev)
> +{
> +	return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
> +}

nit: could you put it in net/core/dev.h ?

Or alternatively rename to netif_isalive(). We should try to avoid using
the dev_ prefix on APIs we export via include/linux/

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

* Re: [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink()
  2026-06-02 17:57 ` Jakub Kicinski
@ 2026-06-02 18:36   ` Eric Dumazet
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2026-06-02 18:36 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S . Miller, Paolo Abeni, Kuniyuki Iwashima, Simon Horman,
	netdev, eric.dumazet

On Tue, Jun 2, 2026 at 10:57 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue,  2 Jun 2026 09:13:19 +0000 Eric Dumazet wrote:
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 74507c006490f180d2fac6594f6dcf2c86d53919..bfc847bc47fd957b58174dc4d2b82e2e8b461405 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -5660,6 +5660,12 @@ static inline const char *netdev_name(const struct net_device *dev)
> >       return dev->name;
> >  }
> >
> > +/* Caller holds RTNL, netdev->lock or RCU */
> > +static inline bool dev_isalive(const struct net_device *dev)
> > +{
> > +     return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
> > +}
>
> nit: could you put it in net/core/dev.h ?
>

Sure thing.

> Or alternatively rename to netif_isalive(). We should try to avoid using
> the dev_ prefix on APIs we export via include/linux/

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

end of thread, other threads:[~2026-06-02 18:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-02  9:13 [PATCH net-next] rtnetlink: use dev_isalive() in rtnl_getlink() Eric Dumazet
2026-06-02 12:23 ` Jiayuan Chen
2026-06-02 12:47   ` Eric Dumazet
2026-06-02 17:57 ` Jakub Kicinski
2026-06-02 18:36   ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox