mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] stddef: make __struct_group() UAPI C++-friendly
@ 2024-12-18 14:57 Alexander Lobakin
  2024-12-18 21:42 ` Kees Cook
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander Lobakin @ 2024-12-18 14:57 UTC (permalink / raw)
  To: Kees Cook
  Cc: Alexander Lobakin, Christopher Ferris, Gustavo A. R. Silva,
	Erick Archer, Keith Packard, Dan Williams, linux-hardening,
	linux-kernel

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
 
 #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
-- 
2.47.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] stddef: make __struct_group() UAPI C++-friendly
  2024-12-18 14:57 [PATCH] stddef: make __struct_group() UAPI C++-friendly Alexander Lobakin
@ 2024-12-18 21:42 ` Kees Cook
  0 siblings, 0 replies; 2+ messages in thread
From: Kees Cook @ 2024-12-18 21:42 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: Christopher Ferris, Gustavo A. R. Silva, Erick Archer,
	Keith Packard, Dan Williams, linux-hardening, linux-kernel



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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-12-18 21:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-18 14:57 [PATCH] stddef: make __struct_group() UAPI C++-friendly Alexander Lobakin
2024-12-18 21:42 ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®