From: Dave Jiang <dave.jiang@intel.com>
To: "Gustavo A. R. Silva" <gustavoars@kernel.org>,
Alison Schofield <alison.schofield@intel.com>,
Dan Williams <dan.j.williams@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Ira Weiny <ira.weiny@intel.com>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Len Brown <lenb@kernel.org>
Cc: nvdimm@lists.linux.dev, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2][next] UAPI: ndctl / acpi: intel: Avoid multiple -Wflex-array-member-not-at-end warnings
Date: Fri, 14 Feb 2025 15:04:52 -0700 [thread overview]
Message-ID: <a66056cf-570c-4875-b5cf-c51e2bc488d9@intel.com> (raw)
In-Reply-To: <Z66T8tSKjVutr6of@kspp>
On 2/13/25 5:53 PM, 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.
>
> Below is the before-and-after changes of the memory layout in `struct
> nd_cmd_pkg`. This to illustrate that the use of `__struct_group()`
> doesn't alter the layout, ensuring that user space remains unaffected.
>
> Before changes:
> struct nd_cmd_pkg {
> __u64 nd_family; /* 0 8 */
> __u64 nd_command; /* 8 8 */
> __u32 nd_size_in; /* 16 4 */
> __u32 nd_size_out; /* 20 4 */
> __u32 nd_reserved2[9]; /* 24 36 */
> __u32 nd_fw_size; /* 60 4 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> unsigned char nd_payload[]; /* 64 0 */
>
> /* size: 64, cachelines: 1, members: 7 */
> };
>
> After changes:
> struct nd_cmd_pkg {
> union {
> struct {
> __u64 nd_family; /* 0 8 */
> __u64 nd_command; /* 8 8 */
> __u32 nd_size_in; /* 16 4 */
> __u32 nd_size_out; /* 20 4 */
> __u32 nd_reserved2[9]; /* 24 36 */
> __u32 nd_fw_size; /* 60 4 */
> }; /* 0 64 */
> struct nd_cmd_pkg_hdr __hdr; /* 0 64 */
> }; /* 0 64 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> unsigned char nd_payload[]; /* 64 0 */
>
> /* size: 64, cachelines: 1, members: 2 */
> };
>
> It's also worth mentioning that all members of the struct can still be
> accessed directly, for example instance->nd_family, instance->nd_command,
> and so on.
>
> So, with these changes, fix 12 of 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]
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> Changes in v2:
> - Show changes in UAPI first. (Alison)
> - Update changelog text --add more information about _struct_group()
> changes. (Alison)
>
> v1:
> - Link: https://lore.kernel.org/linux-hardening/Z618ILbAR8YAvTkd@kspp/
>
> include/uapi/linux/ndctl.h | 15 +++++++++------
> drivers/acpi/nfit/intel.c | 24 ++++++++++++------------
> 2 files changed, 21 insertions(+), 18 deletions(-)
>
> 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
> @@ -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 */
> };
>
> diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
> index 3902759abcba..fe561ce0ddec 100644
> --- a/drivers/acpi/nfit/intel.c
> +++ b/drivers/acpi/nfit/intel.c
> @@ -56,7 +56,7 @@ static unsigned long intel_security_flags(struct nvdimm *nvdimm,
> struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> unsigned long security_flags = 0;
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_get_security_state cmd;
> } nd_cmd = {
> .pkg = {
> @@ -121,7 +121,7 @@ static int intel_security_freeze(struct nvdimm *nvdimm)
> {
> struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_freeze_lock cmd;
> } nd_cmd = {
> .pkg = {
> @@ -154,7 +154,7 @@ static int intel_security_change_key(struct nvdimm *nvdimm,
> NVDIMM_INTEL_SET_MASTER_PASSPHRASE :
> NVDIMM_INTEL_SET_PASSPHRASE;
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_set_passphrase cmd;
> } nd_cmd = {
> .pkg = {
> @@ -196,7 +196,7 @@ static int __maybe_unused intel_security_unlock(struct nvdimm *nvdimm,
> {
> struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_unlock_unit cmd;
> } nd_cmd = {
> .pkg = {
> @@ -235,7 +235,7 @@ static int intel_security_disable(struct nvdimm *nvdimm,
> int rc;
> struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_disable_passphrase cmd;
> } nd_cmd = {
> .pkg = {
> @@ -278,7 +278,7 @@ static int __maybe_unused intel_security_erase(struct nvdimm *nvdimm,
> unsigned int cmd = ptype == NVDIMM_MASTER ?
> NVDIMM_INTEL_MASTER_SECURE_ERASE : NVDIMM_INTEL_SECURE_ERASE;
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_secure_erase cmd;
> } nd_cmd = {
> .pkg = {
> @@ -319,7 +319,7 @@ static int __maybe_unused intel_security_query_overwrite(struct nvdimm *nvdimm)
> int rc;
> struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_query_overwrite cmd;
> } nd_cmd = {
> .pkg = {
> @@ -355,7 +355,7 @@ static int __maybe_unused intel_security_overwrite(struct nvdimm *nvdimm,
> int rc;
> struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_overwrite cmd;
> } nd_cmd = {
> .pkg = {
> @@ -408,7 +408,7 @@ static int intel_bus_fwa_businfo(struct nvdimm_bus_descriptor *nd_desc,
> struct nd_intel_bus_fw_activate_businfo *info)
> {
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_bus_fw_activate_businfo cmd;
> } nd_cmd = {
> .pkg = {
> @@ -519,7 +519,7 @@ static int intel_bus_fwa_activate(struct nvdimm_bus_descriptor *nd_desc)
> {
> struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_bus_fw_activate cmd;
> } nd_cmd = {
> .pkg = {
> @@ -583,7 +583,7 @@ static int intel_fwa_dimminfo(struct nvdimm *nvdimm,
> struct nd_intel_fw_activate_dimminfo *info)
> {
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_fw_activate_dimminfo cmd;
> } nd_cmd = {
> .pkg = {
> @@ -689,7 +689,7 @@ static int intel_fwa_arm(struct nvdimm *nvdimm, enum nvdimm_fwa_trigger arm)
> struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> struct acpi_nfit_desc *acpi_desc = nfit_mem->acpi_desc;
> struct {
> - struct nd_cmd_pkg pkg;
> + struct nd_cmd_pkg_hdr pkg;
> struct nd_intel_fw_activate_arm cmd;
> } nd_cmd = {
> .pkg = {
prev parent reply other threads:[~2025-02-14 22:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-14 0:53 [PATCH v2][next] UAPI: ndctl / acpi: intel: Avoid multiple -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
2025-02-14 1:58 ` Alison Schofield
2025-02-14 22:04 ` Dave Jiang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a66056cf-570c-4875-b5cf-c51e2bc488d9@intel.com \
--to=dave.jiang@intel.com \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=gustavoars@kernel.org \
--cc=ira.weiny@intel.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nvdimm@lists.linux.dev \
--cc=rafael@kernel.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.