All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <edumazet@kernel.org>
To: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>, David Ahern <dsahern@kernel.org>,
	Ido Schimmel <idosch@nvidia.com>,
	Kuniyuki Iwashima <kuniyu@google.com>,
	netdev@vger.kernel.org, Eric Dumazet <edumazet@kernel.org>,
	Baul Lee <baul.lee@xbow.com>
Subject: [PATCH net] ipv4: use RCU protection in inet_netconf_get_devconf()
Date: Fri,  9 Oct 2026 09:49:05 +0200	[thread overview]
Message-ID: <20261009074906.729877-1-edumazet@kernel.org> (raw)

inet_netconf_get_devconf() runs without RTNL. It uses dev_get_by_index()
and in_dev_get() to pin the device and its in_device while the reply is
allocated and filled.

in_dev_get() is not safe for a lockless caller. Since commit 9d40c84cf5bc
("net: devinet: Reduce refcount before grace period"), inetdev_destroy()
drops the final reference before the RCU grace period. A reader can load
dev->ip_ptr, then call refcount_inc() on a zero refcount after
in_dev_free_rcu() has been queued, and keep using the in_device once it
has been freed.

None of these references are needed. An in_device is freed after an RCU
grace period, ipv4_devconf is embedded in it, and
inet_netconf_fill_devconf() does not sleep.

Allocate the skb first, then perform the lookup and the fill under
rcu_read_lock(), using dev_get_by_index_rcu() and __in_dev_get_rcu(),
like inet_netconf_dump_devconf() already does.

Fixes: bbcf91053bb6 ("inet: do not use RTNL in inet_netconf_get_devconf()")
Reported-by: Baul Lee <baul.lee@xbow.com>
Closes: https://lore.kernel.org/netdev/20260815172032.79740-1-baul.lee@xbow.com/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
Sorry for a resend, I did a mistake, using edumazet@google.com instead of
edumazet@kernel.org, not enough sleep I guess.

 net/ipv4/devinet.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 5b6b11c943e453742b4893f1c9bd9e70d3d37a6f..f9a361621f74b420276006b13de94926e13af18e 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2379,8 +2379,8 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
 	struct net *net = sock_net(in_skb->sk);
 	struct nlattr *tb[NETCONFA_MAX + 1];
 	const struct ipv4_devconf *devconf;
-	struct in_device *in_dev = NULL;
-	struct net_device *dev = NULL;
+	struct in_device *in_dev;
+	struct net_device *dev;
 	struct sk_buff *skb;
 	int ifindex;
 	int err;
@@ -2392,7 +2392,13 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
 	if (!tb[NETCONFA_IFINDEX])
 		return -EINVAL;
 
+	skb = nlmsg_new(inet_netconf_msgsize_devconf(NETCONFA_ALL), GFP_KERNEL);
+	if (!skb)
+		return -ENOBUFS;
+
 	ifindex = nla_get_s32(tb[NETCONFA_IFINDEX]);
+
+	rcu_read_lock();
 	switch (ifindex) {
 	case NETCONFA_IFINDEX_ALL:
 		devconf = net->ipv4.devconf_all;
@@ -2402,36 +2408,29 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
 		break;
 	default:
 		err = -ENODEV;
-		dev = dev_get_by_index(net, ifindex);
-		if (dev)
-			in_dev = in_dev_get(dev);
+		dev = dev_get_by_index_rcu(net, ifindex);
+		if (!dev)
+			goto out_unlock;
+		in_dev = __in_dev_get_rcu(dev);
 		if (!in_dev)
-			goto errout;
+			goto out_unlock;
 		devconf = &in_dev->cnf;
 		break;
 	}
 
-	err = -ENOBUFS;
-	skb = nlmsg_new(inet_netconf_msgsize_devconf(NETCONFA_ALL), GFP_KERNEL);
-	if (!skb)
-		goto errout;
-
 	err = inet_netconf_fill_devconf(skb, ifindex, devconf,
 					NETLINK_CB(in_skb).portid,
 					nlh->nlmsg_seq, RTM_NEWNETCONF, 0,
 					NETCONFA_ALL);
+out_unlock:
+	rcu_read_unlock();
 	if (err < 0) {
 		/* -EMSGSIZE implies BUG in inet_netconf_msgsize_devconf() */
 		WARN_ON(err == -EMSGSIZE);
 		kfree_skb(skb);
-		goto errout;
+		return err;
 	}
-	err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
-errout:
-	if (in_dev)
-		in_dev_put(in_dev);
-	dev_put(dev);
-	return err;
+	return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
 }
 
 static int inet_netconf_dump_devconf(struct sk_buff *skb,
-- 
2.53.0


             reply	other threads:[~2026-10-09  7:49 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09  7:49 Eric Dumazet [this message]
2026-10-09  7:54 ` [PATCH net] ipv4: use RCU protection in inet_netconf_get_devconf() netdev-bot+sinfo
  -- strict thread matches above, loose matches on Subject: below --
2026-10-09  7:44 Eric Dumazet
2026-10-09  7:50 ` netdev-bot+sinfo
2026-10-09  9:28 ` Cen Zhang
2026-10-12  8:12 ` Ido Schimmel
2026-10-12 19:58 ` Kuniyuki Iwashima

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261009074906.729877-1-edumazet@kernel.org \
    --to=edumazet@kernel.org \
    --cc=baul.lee@xbow.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.