netdev - [RFC PATCH net-next 06/13] net_failover: hold the netdev lists lock when retrieving device statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20201206235919.393158-7-vladimir.oltean@nxp.com>
Date:   Mon,  7 Dec 2020 01:59:12 +0200
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Paul Gortmaker <paul.gortmaker@...driver.com>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Jiri Benc <jbenc@...hat.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Eric Dumazet <edumazet@...gle.com>,
        George McCollister <george.mccollister@...il.com>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        Sridhar Samudrala <sridhar.samudrala@...el.com>
Subject: [RFC PATCH net-next 06/13] net_failover: hold the netdev lists lock when retrieving device statistics

In the effort of making .ndo_get_stats64 be able to sleep, we need to
ensure the callers of dev_get_stats do not use atomic context.

The net_failover driver makes copious abuse of RCU protection for the
slave interfaces, which is probably unnecessary given the fact that it
already calls dev_hold on slave interfaces. Nonetheless, to avoid
regressions, we still need to offer the same level of protection against
unregistering the standby and primary devices. We can achieve this by
holding the netns mutex, which gives us the sleepable context that
dev_get_stats() wants to see. Holding this mutex also removes the need
for a separate lock for statistics.

Cc: Sridhar Samudrala <sridhar.samudrala@...el.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
 drivers/net/net_failover.c | 15 +++++++--------
 include/net/net_failover.h |  3 ---
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
index 2a4892402ed8..90db0358bc1d 100644
--- a/drivers/net/net_failover.c
+++ b/drivers/net/net_failover.c
@@ -184,32 +184,31 @@ static void net_failover_get_stats(struct net_device *dev,
 {
 	struct net_failover_info *nfo_info = netdev_priv(dev);
 	const struct rtnl_link_stats64 *new;
+	struct net *net = dev_net(dev);
 	struct rtnl_link_stats64 temp;
 	struct net_device *slave_dev;
 
-	spin_lock(&nfo_info->stats_lock);
-	memcpy(stats, &nfo_info->failover_stats, sizeof(*stats));
+	mutex_lock(&net->netdev_lists_lock);
 
-	rcu_read_lock();
+	memcpy(stats, &nfo_info->failover_stats, sizeof(*stats));
 
-	slave_dev = rcu_dereference(nfo_info->primary_dev);
+	slave_dev = nfo_info->primary_dev;
 	if (slave_dev) {
 		new = dev_get_stats(slave_dev, &temp);
 		net_failover_fold_stats(stats, new, &nfo_info->primary_stats);
 		memcpy(&nfo_info->primary_stats, new, sizeof(*new));
 	}
 
-	slave_dev = rcu_dereference(nfo_info->standby_dev);
+	slave_dev = nfo_info->standby_dev;
 	if (slave_dev) {
 		new = dev_get_stats(slave_dev, &temp);
 		net_failover_fold_stats(stats, new, &nfo_info->standby_stats);
 		memcpy(&nfo_info->standby_stats, new, sizeof(*new));
 	}
 
-	rcu_read_unlock();
-
 	memcpy(&nfo_info->failover_stats, stats, sizeof(*stats));
-	spin_unlock(&nfo_info->stats_lock);
+
+	mutex_unlock(&net->netdev_lists_lock);
 }
 
 static int net_failover_change_mtu(struct net_device *dev, int new_mtu)
diff --git a/include/net/net_failover.h b/include/net/net_failover.h
index b12a1c469d1c..1e0089800a28 100644
--- a/include/net/net_failover.h
+++ b/include/net/net_failover.h
@@ -22,9 +22,6 @@ struct net_failover_info {
 
 	/* aggregated stats */
 	struct rtnl_link_stats64 failover_stats;
-
-	/* spinlock while updating stats */
-	spinlock_t stats_lock;
 };
 
 struct failover *net_failover_create(struct net_device *standby_dev);
-- 
2.25.1