netdev - Re: [PATCH 3/3] UAPI: ethtool: Avoid flex-array in struct ethtool_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: <20241115135058.01065c04@kernel.org>
Date: Fri, 15 Nov 2024 13:50:58 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Kees Cook <kees@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>, "David S. Miller"
 <davem@...emloft.net>, Andrew Lunn <andrew@...n.ch>, "Kory Maincent (Dent
 Project)" <kory.maincent@...tlin.com>, Michael Chan
 <michael.chan@...adcom.com>, Andrew Lunn <andrew+netdev@...n.ch>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Potnuri
 Bharat Teja <bharat@...lsio.com>, Christian Benvenuti <benve@...co.com>,
 Satish Kharat <satishkh@...co.com>, Manish Chopra <manishc@...vell.com>,
 Simon Horman <horms@...nel.org>, Edward Cree <ecree.xilinx@...il.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>, Heiner Kallweit
 <hkallweit1@...il.com>, Ahmed Zaki <ahmed.zaki@...el.com>, Rahul Rameshbabu
 <rrameshbabu@...dia.com>, Ido Schimmel <idosch@...dia.com>, Maxime
 Chevallier <maxime.chevallier@...tlin.com>, Takeru Hayasaka
 <hayatake396@...il.com>, Jonathan Corbet <corbet@....net>,
 linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject: Re: [PATCH 3/3] UAPI: ethtool: Avoid flex-array in struct
 ethtool_link_settings

On Fri, 15 Nov 2024 12:43:05 -0800 Kees Cook wrote:
> struct ethtool_link_settings tends to be used as a header for other
> structures that have trailing bytes[1], but has a trailing flexible array
> itself. Using this overlapped with other structures leads to ambiguous
> object sizing in the compiler, so we want to avoid such situations (which
> have caused real bugs in the past). Detecting this can be done with
> -Wflex-array-member-not-at-end, which will need to be enabled globally.
> 
> Using a tagged struct_group() to create a new ethtool_link_settings_hdr
> structure isn't possible as it seems we cannot use the tagged variant of
> struct_group() due to syntax issues from C++'s perspective (even within
> "extern C")[2]. Instead, we can just leave the offending member defined
> in UAPI and remove it from the kernel's view of the structure, as Linux
> doesn't actually use this member at all. There is also no change in
> size since it was already a flexible array that didn't contribute to
> size returned by any use of sizeof().

Perfect. I was starting to doubt if user space needs the member,
ethtool CLI doesn't but looks like NetworkManager does. 
So as you say we'll cross that bridge...

Reviewed-by: Jakub Kicinski <kuba@...nel.org>

Thanks!