linux-kernel - Re: [PATCH net-next v3] netdevice: define and allocate &net_device _properly

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: <202407100927.3CA9DE888A@keescook>
Date: Wed, 10 Jul 2024 09:29:46 -0700
From: Kees Cook <kees@...nel.org>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Breno Leitao <leitao@...ian.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>, horms@...nel.org,
	linux-hardening@...r.kernel.org,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Daniel Borkmann <daniel@...earbox.net>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Johannes Berg <johannes.berg@...el.com>,
	"open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v3] netdevice: define and allocate &net_device
 _properly_

On Wed, Jul 10, 2024 at 04:01:35PM +0200, Alexander Lobakin wrote:
> From: Breno Leitao <leitao@...ian.org>
> Date: Wed, 10 Jul 2024 04:30:28 -0700
> 
> > From: Alexander Lobakin <aleksander.lobakin@...el.com>
> > 
> > In fact, this structure contains a flexible array at the end, but
> > historically its size, alignment etc., is calculated manually.
> > There are several instances of the structure embedded into other
> > structures, but also there's ongoing effort to remove them and we
> > could in the meantime declare &net_device properly.
> > Declare the array explicitly, use struct_size() and store the array
> > size inside the structure, so that __counted_by() can be applied.
> > Don't use PTR_ALIGN(), as SLUB itself tries its best to ensure the
> > allocated buffer is aligned to what the user expects.
> > Also, change its alignment from %NETDEV_ALIGN to the cacheline size
> > as per several suggestions on the netdev ML.
> > 
> > bloat-o-meter for vmlinux:
> > 
> > free_netdev                                  445     440      -5
> > netdev_freemem                                24       -     -24
> > alloc_netdev_mqs                            1481    1450     -31
> > 
> > On x86_64 with several NICs of different vendors, I was never able to
> > get a &net_device pointer not aligned to the cacheline size after the
> > change.
> > 
> > Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> > Signed-off-by: Breno Leitao <leitao@...ian.org>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> > Reviewed-by: Eric Dumazet <edumazet@...gle.com>
> > Reviewed-by: Kees Cook <kees@...nel.org>
> 
> You did a great job converting embedded &net_devices, thanks a lot!
> 
> I hope SLUB won't return you a non-cacheline-aligned pointer after that
> you removed SMP_CACHE_ALIGN(sizeof_priv), right?

Currently the slab will do power-of-2 alignment (i.e. aligned to the
bucket size), so this should be fine. In the future I'm trying to make
the slab more aware of the required alignments so that it can still
provide needed alignment without having to do maximal (power-of-2)
alignments.

-- 
Kees Cook