linux-hardening - [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: <20241115204308.3821419-3-kees@kernel.org>
Date: Fri, 15 Nov 2024 12:43:05 -0800
From: Kees Cook <kees@...nel.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Kees Cook <kees@...nel.org>,
	"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: [PATCH 3/3] UAPI: ethtool: Avoid flex-array in struct ethtool_link_settings

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().

Reported-by: Jakub Kicinski <kuba@...nel.org>
Closes: https://lore.kernel.org/lkml/20241109100213.262a2fa0@kernel.org/ [2]
Link: https://lore.kernel.org/lkml/0bc2809fe2a6c11dd4c8a9a10d9bd65cccdb559b.1730238285.git.gustavoars@kernel.org/ [1]
Signed-off-by: Kees Cook <kees@...nel.org>
---
Cc: Jakub Kicinski <kuba@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>
Cc: Andrew Lunn <andrew@...n.ch>
Cc: "Kory Maincent (Dent Project)" <kory.maincent@...tlin.com>
---
 include/uapi/linux/ethtool.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index c405ed63acfa..7e1b3820f91f 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -2526,12 +2526,19 @@ struct ethtool_link_settings {
 	__u8	master_slave_state;
 	__u8	rate_matching;
 	__u32	reserved[7];
+#ifndef __KERNEL__
+	/* Linux builds with -Wflex-array-member-not-at-end but does
+	 * not use the "link_mode_masks" member. Leave it defined for
+	 * userspace for now, and when userspace wants to start using
+	 * -Wfamnae, we'll need a new solution.
+	 */
 	__u32	link_mode_masks[];
 	/* layout of link_mode_masks fields:
 	 * __u32 map_supported[link_mode_masks_nwords];
 	 * __u32 map_advertising[link_mode_masks_nwords];
 	 * __u32 map_lp_advertising[link_mode_masks_nwords];
 	 */
+#endif
 };
 
 /**
-- 
2.34.1