Re: [PATCH] stddef: make __struct_group() UAPI C++-friendly

[Kees Cook <kees@kernel.org>]

On December 18, 2024 6:57:58 AM PST, Alexander Lobakin <aleksander.lobakin@intel.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@google.com>
>Closes: https://lore.kernel.org/linux-hardening/Z1HZpe3WE5As8UAz@google.com
>Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.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