All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Alison Schofield <alison.schofield@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Dave Jiang <dave.jiang@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, LIBNVDIMM: ;,
	NON-VOLATILE MEMORY DEVICE SUBSYSTEM status: ;,
	SupportedACPI status:  "SupportedGustavo A. R. Silva"
	<gustavoars@kernel.org>, linux-hardening@vger.kernel.org;
Subject: [PATCH v2][next] UAPI: ndctl / acpi: intel: Avoid multiple -Wflex-array-member-not-at-end warnings
Date: Fri, 14 Feb 2025 11:23:06 +1030	[thread overview]
Message-ID: <Z66T8tSKjVutr6of@kspp> (raw)

-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>
---
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 = {
-- 
2.43.0


             reply	other threads:[~2025-02-14  0:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-14  0:53 Gustavo A. R. Silva [this message]
2025-02-14  1:58 ` [PATCH v2][next] UAPI: ndctl / acpi: intel: Avoid multiple -Wflex-array-member-not-at-end warnings Alison Schofield
2025-02-14 22:04 ` Dave Jiang

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=Z66T8tSKjVutr6of@kspp \
    --to=gustavoars@kernel.org \
    --cc=alison.schofield@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --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.