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

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

Message-ID: <f4947447-aa66-470c-a48d-06ed77be58da@intel.com>
Date: Tue, 17 Dec 2024 15:55:41 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Kees Cook <kees@...nel.org>
CC: Jakub Kicinski <kuba@...nel.org>, <cferris@...gle.com>, Jamal Hadi Salim
	<jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>, Jiri Pirko
	<jiri@...nulli.us>, <netdev@...r.kernel.org>, "Gustavo A. R. Silva"
	<gustavoars@...nel.org>, <linux-kernel@...r.kernel.org>,
	<linux-hardening@...r.kernel.org>
Subject: Re: [PATCH] UAPI: net/sched: Open-code __struct_group() in flex
 struct tc_u32_sel

From: Kees Cook <kees@...nel.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@...gle.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@...nel.org>
> ---
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Jamal Hadi Salim <jhs@...atatu.com>
> Cc: Cong Wang <xiyou.wangcong@...il.com>
> Cc: Jiri Pirko <jiri@...nulli.us>
> Cc: netdev@...r.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