diff mbox series

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

Message ID 20241219135734.2130002-1-aleksander.lobakin@intel.com (mailing list archive)
State New
Headers show
Series [v2] stddef: make __struct_group() UAPI C++-friendly | expand

Commit Message

Alexander Lobakin Dec. 19, 2024, 1:57 p.m. UTC
For the most part of the 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
Suggested-by: Kees Cook <kees@kernel.org> # __struct_group_tag()
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
From v1[0]:
* wrap @TAG into __struct_group_tag() (Kees).

[0] https://lore.kernel.org/linux-hardening/20241218145758.701008-1-aleksander.lobakin@intel.com
---
 include/uapi/linux/stddef.h       | 13 ++++++++++---
 tools/include/uapi/linux/stddef.h | 15 +++++++++++----
 2 files changed, 21 insertions(+), 7 deletions(-)

Comments

Gustavo A. R. Silva Dec. 19, 2024, 3:41 p.m. UTC | #1
On 19/12/24 07:57, Alexander Lobakin wrote:
> For the most part of the 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
> Suggested-by: Kees Cook <kees@kernel.org> # __struct_group_tag()
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks!
-Gustavo

> ---
>  From v1[0]:
> * wrap @TAG into __struct_group_tag() (Kees).
> 
> [0] https://lore.kernel.org/linux-hardening/20241218145758.701008-1-aleksander.lobakin@intel.com
> ---
>   include/uapi/linux/stddef.h       | 13 ++++++++++---
>   tools/include/uapi/linux/stddef.h | 15 +++++++++++----
>   2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
> index 58154117d9b0..a6fce46aeb37 100644
> --- a/include/uapi/linux/stddef.h
> +++ b/include/uapi/linux/stddef.h
> @@ -8,6 +8,13 @@
>   #define __always_inline inline
>   #endif
>   
> +/* Not all C++ standards support type declarations inside an anonymous union */
> +#ifndef __cplusplus
> +#define __struct_group_tag(TAG)		TAG
> +#else
> +#define __struct_group_tag(TAG)
> +#endif
> +
>   /**
>    * __struct_group() - Create a mirrored named and anonyomous struct
>    *
> @@ -20,13 +27,13 @@
>    * 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.
>    */
>   #define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
>   	union { \
>   		struct { MEMBERS } ATTRS; \
> -		struct TAG { MEMBERS } ATTRS NAME; \
> +		struct __struct_group_tag(TAG) { MEMBERS } ATTRS NAME; \
>   	} ATTRS
>   
>   #ifdef __cplusplus
> diff --git a/tools/include/uapi/linux/stddef.h b/tools/include/uapi/linux/stddef.h
> index bb6ea517efb5..c53cde425406 100644
> --- a/tools/include/uapi/linux/stddef.h
> +++ b/tools/include/uapi/linux/stddef.h
> @@ -8,6 +8,13 @@
>   #define __always_inline __inline__
>   #endif
>   
> +/* Not all C++ standards support type declarations inside an anonymous union */
> +#ifndef __cplusplus
> +#define __struct_group_tag(TAG)		TAG
> +#else
> +#define __struct_group_tag(TAG)
> +#endif
> +
>   /**
>    * __struct_group() - Create a mirrored named and anonyomous struct
>    *
> @@ -20,14 +27,14 @@
>    * 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.
>    */
>   #define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
>   	union { \
>   		struct { MEMBERS } ATTRS; \
> -		struct TAG { MEMBERS } ATTRS NAME; \
> -	}
> +		struct __struct_group_tag(TAG) { MEMBERS } ATTRS NAME; \
> +	} ATTRS
>   
>   /**
>    * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union
Kees Cook Dec. 20, 2024, 5:09 p.m. UTC | #2
On Thu, 19 Dec 2024 14:57:34 +0100, Alexander Lobakin wrote:
> For the most part of the 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]
> 
> [...]

Applied to for-next/hardening, thanks!

[1/1] stddef: make __struct_group() UAPI C++-friendly
      https://git.kernel.org/kees/c/724c6ce38bba

Take care,
diff mbox series

Patch

diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
index 58154117d9b0..a6fce46aeb37 100644
--- a/include/uapi/linux/stddef.h
+++ b/include/uapi/linux/stddef.h
@@ -8,6 +8,13 @@ 
 #define __always_inline inline
 #endif
 
+/* Not all C++ standards support type declarations inside an anonymous union */
+#ifndef __cplusplus
+#define __struct_group_tag(TAG)		TAG
+#else
+#define __struct_group_tag(TAG)
+#endif
+
 /**
  * __struct_group() - Create a mirrored named and anonyomous struct
  *
@@ -20,13 +27,13 @@ 
  * 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.
  */
 #define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
 	union { \
 		struct { MEMBERS } ATTRS; \
-		struct TAG { MEMBERS } ATTRS NAME; \
+		struct __struct_group_tag(TAG) { MEMBERS } ATTRS NAME; \
 	} ATTRS
 
 #ifdef __cplusplus
diff --git a/tools/include/uapi/linux/stddef.h b/tools/include/uapi/linux/stddef.h
index bb6ea517efb5..c53cde425406 100644
--- a/tools/include/uapi/linux/stddef.h
+++ b/tools/include/uapi/linux/stddef.h
@@ -8,6 +8,13 @@ 
 #define __always_inline __inline__
 #endif
 
+/* Not all C++ standards support type declarations inside an anonymous union */
+#ifndef __cplusplus
+#define __struct_group_tag(TAG)		TAG
+#else
+#define __struct_group_tag(TAG)
+#endif
+
 /**
  * __struct_group() - Create a mirrored named and anonyomous struct
  *
@@ -20,14 +27,14 @@ 
  * 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.
  */
 #define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
 	union { \
 		struct { MEMBERS } ATTRS; \
-		struct TAG { MEMBERS } ATTRS NAME; \
-	}
+		struct __struct_group_tag(TAG) { MEMBERS } ATTRS NAME; \
+	} ATTRS
 
 /**
  * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union