linux-kernel - Re: [PATCH][next] UAPI: ndctl / acpi: intel: Avoid multiple -Wflex-array-member-not-at-end warnings

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

Message-ID: <Z65xV2TaFY32eYdp@aschofie-mobl2.lan>
Date: Thu, 13 Feb 2025 14:25:27 -0800
From: Alison Schofield <alison.schofield@...el.com>
To: "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: Dan Williams <dan.j.williams@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>,
	Dave Jiang <dave.jiang@...el.com>, Ira Weiny <ira.weiny@...el.com>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Len Brown <lenb@...nel.org>, nvdimm@...ts.linux.dev,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] UAPI: ndctl / acpi: intel: Avoid multiple
 -Wflex-array-member-not-at-end warnings

On Thu, Feb 13, 2025 at 03:29:12PM +1030, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
> 
> So, in order to avoid ending up with flexible-array members in the
> middle of other structs, we use the `__struct_group()` helper to
> separate the flexible array from the rest of the members in the
> flexible structure. We then use the newly created tagged `struct
> nd_cmd_pkg_hdr` to replace the type of the objects causing trouble
> (`pkg`) in multiple structs.
> 
> So, with these changes, fix the following warnings:
> 
> drivers/acpi/nfit/intel.c:692:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

One sample warning is good.

How about adding a comment on why the usage of __struct_group() here
means this doesn't break userspace.

snip

> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> index 73516e263627..34c11644d5d7 100644
> --- a/include/uapi/linux/ndctl.h
> +++ b/include/uapi/linux/ndctl.h

FWIW: In a patch like this where reviewers likely want to see
the header file change first, put it first in the diff.
(git diff.orderfile)


> @@ -227,12 +227,15 @@ enum ars_masks {
>   */
>  
>  struct nd_cmd_pkg {
> -	__u64   nd_family;		/* family of commands */
> -	__u64   nd_command;
> -	__u32   nd_size_in;		/* INPUT: size of input args */
> -	__u32   nd_size_out;		/* INPUT: size of payload */
> -	__u32   nd_reserved2[9];	/* reserved must be zero */
> -	__u32   nd_fw_size;		/* OUTPUT: size fw wants to return */
> +	/* New members MUST be added within the __struct_group() macro below. */
> +	__struct_group(nd_cmd_pkg_hdr, __hdr, /* no attrs */,
> +		__u64   nd_family;		/* family of commands */
> +		__u64   nd_command;
> +		__u32   nd_size_in;		/* INPUT: size of input args */
> +		__u32   nd_size_out;		/* INPUT: size of payload */
> +		__u32   nd_reserved2[9];	/* reserved must be zero */
> +		__u32   nd_fw_size;		/* OUTPUT: size fw wants to return */
> +	);
>  	unsigned char nd_payload[];	/* Contents of call      */
>  };
>  
> -- 
> 2.43.0
> 
>