The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net] ipv4: Fix in_device refcount resurrection in in_dev_get()
@ 2026-08-15 17:20 Baul Lee
  2026-08-17 16:00 ` Ido Schimmel
  0 siblings, 1 reply; 2+ messages in thread
From: Baul Lee @ 2026-08-15 17:20 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, dsahern, idosch
  Cc: jiri, netdev, linux-kernel, federico.kirschbaum

in_dev_get() reads dev->ip_ptr under RCU and then unconditionally
increments its refcount. inetdev_destroy() clears the pointer and drops
the last reference under RTNL, with no grace period in between, so a
reader that fetched the pointer before the store can increment a
refcount that has already reached zero. That resurrects an object whose
RCU free is queued: dropping the resurrected reference re-enters
in_dev_finish_destroy() for a second netdev_put() and a second
call_rcu() on the same rcu_head, and if the grace period elapses first
the drop itself is a use-after-free.

inet_netconf_get_devconf() is registered RTNL_FLAG_DOIT_UNLOCKED, and
rtnetlink_rcv_msg() exempts RTNL_KIND_GET from the CAP_NET_ADMIN check,
so an unprivileged user can drive the reader side. Reproduced as UID
65534 on v7.2-rc7:

  refcount_t: addition on 0; use-after-free.
  WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x14c/0x180
  CPU: 0 UID: 65534 PID: 655 Comm: j1_poc
   refcount_warn_saturate+0x14c/0x180 (P)
   inet_netconf_get_devconf+0x4b0/0x4c4
   rtnetlink_rcv_msg+0x434/0x4d0

followed by the matching underflow when the reference is dropped.

Use refcount_inc_not_zero() and return NULL for an in_device that has
already reached zero. All callers already handle a NULL return, which
in_dev_get() gives today whenever dev->ip_ptr is NULL. Callers under
RTNL see no change: ip_ptr is cleared before the last put, so a non-NULL
ip_ptr there implies a non-zero refcount.

Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>

Fixes: bbcf91053bb6 ("inet: do not use RTNL in inet_netconf_get_devconf()")
Signed-off-by: Baul Lee <baul.lee@xbow.com>
---
 include/linux/inetdevice.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 6032eea2539a..a1446da64200 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -245,8 +245,8 @@ static inline struct in_device *in_dev_get(const struct net_device *dev)
 
 	rcu_read_lock();
 	in_dev = __in_dev_get_rcu(dev);
-	if (in_dev)
-		refcount_inc(&in_dev->refcnt);
+	if (in_dev && !refcount_inc_not_zero(&in_dev->refcnt))
+		in_dev = NULL;
 	rcu_read_unlock();
 	return in_dev;
 }
-- 
2.50.1


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

* Re: [PATCH net] ipv4: Fix in_device refcount resurrection in in_dev_get()
  2026-08-15 17:20 [PATCH net] ipv4: Fix in_device refcount resurrection in in_dev_get() Baul Lee
@ 2026-08-17 16:00 ` Ido Schimmel
  0 siblings, 0 replies; 2+ messages in thread
From: Ido Schimmel @ 2026-08-17 16:00 UTC (permalink / raw)
  To: Baul Lee
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, dsahern, jiri,
	netdev, linux-kernel, federico.kirschbaum

On Sun, Aug 16, 2026 at 02:20:32AM +0900, Baul Lee wrote:
> in_dev_get() reads dev->ip_ptr under RCU and then unconditionally
> increments its refcount. inetdev_destroy() clears the pointer and drops
> the last reference under RTNL, with no grace period in between, so a
> reader that fetched the pointer before the store can increment a
> refcount that has already reached zero. That resurrects an object whose
> RCU free is queued:

This part is fine.

> dropping the resurrected reference re-enters in_dev_finish_destroy()
> for a second netdev_put() and a second call_rcu() on the same
> rcu_head, and if the grace period elapses first the drop itself is a
> use-after-free.

Are you sure about this part? The reference count is set to
REFCOUNT_SATURATED when you increment from zero, so I don't think you
re-enter in_dev_finish_destroy().

> 
> inet_netconf_get_devconf() is registered RTNL_FLAG_DOIT_UNLOCKED, and
> rtnetlink_rcv_msg() exempts RTNL_KIND_GET from the CAP_NET_ADMIN check,
> so an unprivileged user can drive the reader side. Reproduced as UID
> 65534 on v7.2-rc7:
> 
>   refcount_t: addition on 0; use-after-free.
>   WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x14c/0x180
>   CPU: 0 UID: 65534 PID: 655 Comm: j1_poc
>    refcount_warn_saturate+0x14c/0x180 (P)
>    inet_netconf_get_devconf+0x4b0/0x4c4
>    rtnetlink_rcv_msg+0x434/0x4d0
> 
> followed by the matching underflow when the reference is dropped.
> 
> Use refcount_inc_not_zero() and return NULL for an in_device that has
> already reached zero. All callers already handle a NULL return, which
> in_dev_get() gives today whenever dev->ip_ptr is NULL. Callers under
> RTNL see no change: ip_ptr is cleared before the last put, so a non-NULL
> ip_ptr there implies a non-zero refcount.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>

FYI, it was actually mentioned a few times already:

https://lore.kernel.org/netdev/20260802115639.GA270646@shredder/
https://sashiko.dev/#/patchset/20260731135202.566337-1-david.lee%40trailofbits.com

> 
> Fixes: bbcf91053bb6 ("inet: do not use RTNL in inet_netconf_get_devconf()")

I think you should blame commit 9d40c84cf5bc ("net: devinet: Reduce
refcount before grace period") instead:

1. in_dev_get() was called w/o RTNL even before bbcf91053bb6.

2. Calling in_dev_get() w/o RTNL only became unsafe after 9d40c84cf5bc.
Before that, inetdev_destroy() dropped the reference after an RCU grace
period.

The diff itself looks OK.

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

end of thread, other threads:[~2026-08-17 16:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-15 17:20 [PATCH net] ipv4: Fix in_device refcount resurrection in in_dev_get() Baul Lee
2026-08-17 16:00 ` Ido Schimmel

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