netdev - Re: [PATCH net-next 12/13] rtnetlink: make rtnl_fill_link

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: <ZdYes3iPqzf0FCTf@nanopsycho>
Date: Wed, 21 Feb 2024 17:02:59 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S . Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH net-next 12/13] rtnetlink: make rtnl_fill_link_ifmap()
 RCU ready

Wed, Feb 21, 2024 at 11:59:14AM CET, edumazet@...gle.com wrote:
>Use READ_ONCE() to read the following device fields:
>
>	dev->mem_start
>	dev->mem_end
>	dev->base_addr
>	dev->irq
>	dev->dma
>	dev->if_port
>
>Provide IFLA_MAP attribute only if at least one of these fields
>is not zero. This saves some space in the output skb for most devices.
>
>Signed-off-by: Eric Dumazet <edumazet@...gle.com>
>---
> net/core/rtnetlink.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
>diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>index 1b26dfa5668d22fb2e30ceefbf143e98df13ae29..b91ec216c593aaebf97ea69aa0d2d265ab61c098 100644
>--- a/net/core/rtnetlink.c
>+++ b/net/core/rtnetlink.c
>@@ -1455,19 +1455,21 @@ static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
> 	return 0;
> }
> 
>-static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
>+static int rtnl_fill_link_ifmap(struct sk_buff *skb,
>+				const struct net_device *dev)
> {
> 	struct rtnl_link_ifmap map;
> 
> 	memset(&map, 0, sizeof(map));
>-	map.mem_start   = dev->mem_start;
>-	map.mem_end     = dev->mem_end;
>-	map.base_addr   = dev->base_addr;
>-	map.irq         = dev->irq;
>-	map.dma         = dev->dma;
>-	map.port        = dev->if_port;
>-
>-	if (nla_put_64bit(skb, IFLA_MAP, sizeof(map), &map, IFLA_PAD))
>+	map.mem_start = READ_ONCE(dev->mem_start);
>+	map.mem_end   = READ_ONCE(dev->mem_end);
>+	map.base_addr = READ_ONCE(dev->base_addr);
>+	map.irq       = READ_ONCE(dev->irq);
>+	map.dma       = READ_ONCE(dev->dma);
>+	map.port      = READ_ONCE(dev->if_port);
>+	/* Only report non zero information. */
>+	if (memchr_inv(&map, 0, sizeof(map)) &&

This check(optimization) is unrelated to the rest of the patch, correct?
If yes, could it be a separate patch?


>+	    nla_put_64bit(skb, IFLA_MAP, sizeof(map), &map, IFLA_PAD))
> 		return -EMSGSIZE;
> 
> 	return 0;
>@@ -1875,9 +1877,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
> 			goto nla_put_failure;
> 	}
> 
>-	if (rtnl_fill_link_ifmap(skb, dev))
>-		goto nla_put_failure;
>-
> 	if (dev->addr_len) {
> 		if (nla_put(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr) ||
> 		    nla_put(skb, IFLA_BROADCAST, dev->addr_len, dev->broadcast))
>@@ -1927,6 +1926,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
> 	rcu_read_lock();
> 	if (rtnl_fill_link_af(skb, dev, ext_filter_mask))
> 		goto nla_put_failure_rcu;
>+	if (rtnl_fill_link_ifmap(skb, dev))
>+		goto nla_put_failure_rcu;
>+
> 	rcu_read_unlock();
> 
> 	if (rtnl_fill_prop_list(skb, dev))
>-- 
>2.44.0.rc0.258.g7320e95886-goog
>
>