linux-kernel - Re: [PATCH] stddef: make __struct

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] [day] [month] [year] [list]

Message-ID: <57A07BD9-8EAD-4AC9-91B5-7F6D3850BB9C@kernel.org>
Date: Wed, 18 Dec 2024 13:42:40 -0800
From: Kees Cook <kees@...nel.org>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
CC: Christopher Ferris <cferris@...gle.com>,
 "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 Erick Archer <erick.archer@...look.com>, Keith Packard <keithp@...thp.com>,
 Dan Williams <dan.j.williams@...el.com>, linux-hardening@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] stddef: make __struct_group() UAPI C++-friendly



On December 18, 2024 6:57:58 AM PST, Alexander Lobakin <aleksander.lobakin@...el.com> wrote:
>For the most part of C++ history, it couldn't have type declarations
>inside anonymous unions for different reasons. At the same time,
>__struct_group() relies on the latters, so when the @TAG argument
>is not empty, C++ code doesn't want to build (even under
>`extern "C"`):
>
>../linux/include/uapi/linux/pkt_cls.h:25:24: error:
>'struct tc_u32_sel::<unnamed union>::tc_u32_sel_hdr,' invalid;
>an anonymous union may only have public non-static data members
>[-fpermissive]
>
>The safest way to fix this without trying to switch standards (which
>is impossible in UAPI anyway) etc., is to disable tag declaration
>for that language. This won't break anything since for now it's not
>buildable at all.
>Use a separate definition for __struct_group() when __cplusplus is
>defined to mitigate the error, including the version from tools/.
>
>Fixes: 50d7bd38c3aa ("stddef: Introduce struct_group() helper macro")
>Reported-by: Christopher Ferris <cferris@...gle.com>
>Closes: https://lore.kernel.org/linux-hardening/Z1HZpe3WE5As8UAz@google.com
>Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
>---
> include/uapi/linux/stddef.h       | 12 ++++++++++--
> tools/include/uapi/linux/stddef.h | 14 +++++++++++---
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
>diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
>index 58154117d9b0..ad51b1362bbb 100644
>--- a/include/uapi/linux/stddef.h
>+++ b/include/uapi/linux/stddef.h
>@@ -20,14 +20,22 @@
>  * and size: one anonymous and one named. The former's members can be used
>  * normally without sub-struct naming, and the latter can be used to
>  * reason about the start, end, and size of the group of struct members.
>- * The named struct can also be explicitly tagged for layer reuse, as well
>- * as both having struct attributes appended.
>+ * The named struct can also be explicitly tagged for layer reuse (C only),
>+ * as well as both having struct attributes appended.
>  */
>+#ifndef __cplusplus
> #define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
> 	union { \
> 		struct { MEMBERS } ATTRS; \
> 		struct TAG { MEMBERS } ATTRS NAME; \
> 	} ATTRS
>+#else
>+#define __struct_group(__IGNORED, NAME, ATTRS, MEMBERS...) \
>+	union { \
>+		struct { MEMBERS } ATTRS; \
>+		struct { MEMBERS } ATTRS NAME; \
>+	} ATTRS
>+#endif

Instead of copying the entire define, what about just wrapping the tag?

#ifdef __cplusplus
#define __struct_group_tag(TAG)
#else
#define __struct_group_tag(TAG) TAG
#endif

#define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
 	union { \
 		struct { MEMBERS } ATTRS; \
 		struct __struct_group_tag(TAG) { MEMBERS } ATTRS NAME; \
 	} ATTRS

-Kees

> 
> #ifdef __cplusplus
> /* sizeof(struct{}) is 1 in C++, not 0, can't use C version of the macro. */
>diff --git a/tools/include/uapi/linux/stddef.h b/tools/include/uapi/linux/stddef.h
>index bb6ea517efb5..12b854ecc215 100644
>--- a/tools/include/uapi/linux/stddef.h
>+++ b/tools/include/uapi/linux/stddef.h
>@@ -20,14 +20,22 @@
>  * and size: one anonymous and one named. The former's members can be used
>  * normally without sub-struct naming, and the latter can be used to
>  * reason about the start, end, and size of the group of struct members.
>- * The named struct can also be explicitly tagged for layer reuse, as well
>- * as both having struct attributes appended.
>+ * The named struct can also be explicitly tagged for layer reuse (C only),
>+ * as well as both having struct attributes appended.
>  */
>+#ifndef __cplusplus
> #define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
> 	union { \
> 		struct { MEMBERS } ATTRS; \
> 		struct TAG { MEMBERS } ATTRS NAME; \
>-	}
>+	} ATTRS
>+#else
>+#define __struct_group(__IGNORED, NAME, ATTRS, MEMBERS...) \
>+	union { \
>+		struct { MEMBERS } ATTRS; \
>+		struct { MEMBERS } ATTRS NAME; \
>+	} ATTRS
>+#endif
> 
> /**
>  * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union

-- 
Kees Cook