Re: [PATCH] UAPI: net/sched: Open-code __struct_group() in flex struct tc_u32_sel

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Kees Cook <kees@kernel.org>
Date: Mon, 16 Dec 2024 18:59:55 -0800

> This switches to using a manually constructed form of struct tagging
> to avoid issues with C++ being unable to parse tagged structs within
> anonymous unions, even under 'extern "C"':
> 
>   ../linux/include/uapi/linux/pkt_cls.h:25124: error: ‘struct tc_u32_sel::<unnamed union>::tc_u32_sel_hdr,’ invalid; an anonymous union may only have public non-static data members [-fpermissive]

I worked around that like this in the past: [0]
As I'm not sure it would be fine to fix every such occurrence manually
by open-coding.
What do you think?

> 
> To avoid having multiple struct member lists, use a define to declare
> them.
> 
> Reported-by: cferris@google.com
> Closes: https://lore.kernel.org/linux-hardening/Z1HZpe3WE5As8UAz@google.com/
> Fixes: 216203bdc228 ("UAPI: net/sched: Use __struct_group() in flex struct tc_u32_sel")
> Link: https://lore.kernel.org/r/202412120927.943DFEDD@keescook
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Cong Wang <xiyou.wangcong@gmail.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: netdev@vger.kernel.org
> ---
>  include/uapi/linux/pkt_cls.h | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 2c32080416b5..02aee6ed6bf0 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -245,20 +245,28 @@ struct tc_u32_key {
>  	int		offmask;
>  };
>  
> +#define tc_u32_sel_hdr_members			\
> +	unsigned char		flags;		\
> +	unsigned char		offshift;	\
> +	unsigned char		nkeys;		\
> +	__be16			offmask;	\
> +	__u16			off;		\
> +	short			offoff;		\
> +	short			hoff;		\
> +	__be32			hmask
> +
> +struct tc_u32_sel_hdr {
> +	tc_u32_sel_hdr_members;
> +};
> +
>  struct tc_u32_sel {
> -	/* New members MUST be added within the __struct_group() macro below. */
> -	__struct_group(tc_u32_sel_hdr, hdr, /* no attrs */,
> -		unsigned char		flags;
> -		unsigned char		offshift;
> -		unsigned char		nkeys;
> -
> -		__be16			offmask;
> -		__u16			off;
> -		short			offoff;
> -
> -		short			hoff;
> -		__be32			hmask;
> -	);
> +	/* Open-coded struct_group() to avoid C++ errors. */
> +	union {
> +		struct tc_u32_sel_hdr hdr;
> +		struct {
> +			tc_u32_sel_hdr_members;
> +		};
> +	};
>  	struct tc_u32_key	keys[];
>  };

[0]
https://github.com/alobakin/linux/commit/2a065c7bae821f5fa85fff6f97fbbd460f4aa0f3

Thanks,
Olek