From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>,
Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: linux-arm-msm@vger.kernel.org, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
linux-hardening@vger.kernel.org
Subject: [PATCH][next] rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings
Date: Wed, 7 Aug 2024 09:19:07 -0600 [thread overview]
Message-ID: <ZrOQa2gew5yadyt3@cute> (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 a flexible-array member in the
middle of multiple other structs, we use the `__struct_group()`
helper to create a new tagged `struct glink_msg_hdr`. This structure
groups together all the members of the flexible `struct glink_msg`
except the flexible array.
As a result, the array is effectively separated from the rest of the
members without modifying the memory layout of the flexible structure.
We then change the type of the middle struct members currently causing
trouble from `struct glink_msg` to `struct glink_msg_hdr`.
We also want to ensure that when new members need to be added to the
flexible structure, they are always included within the newly created
tagged struct. For this, we use `static_assert()`. This ensures that the
memory layout for both the flexible structure and the new tagged struct
is the same after any changes.
This approach avoids having to implement `struct glink_msg_hdr` as a
completely separate structure, thus preventing having to maintain two
independent but basically identical structures, closing the door to
potential bugs in the future.
We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure, through which we can access the flexible-array
member, if necessary.
Additionally, we use the `DEFINE_RAW_FLEX()` helper for an on-stack
definition of a flexible structure where the size for the flexible-array
member is known at compile-time.
So, with these changes, fix the following warnings:
drivers/rpmsg/qcom_glink_native.c:51:26: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:459:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:846:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:968:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:1380:34: 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>
---
drivers/rpmsg/qcom_glink_native.c | 42 +++++++++++++++++--------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 82d460ff4777..ed89b810f262 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -30,11 +30,16 @@
#define RPM_GLINK_CID_MAX 65536
struct glink_msg {
- __le16 cmd;
- __le16 param1;
- __le32 param2;
+ /* New members MUST be added within the __struct_group() macro below. */
+ __struct_group(glink_msg_hdr, hdr, __packed,
+ __le16 cmd;
+ __le16 param1;
+ __le32 param2;
+ );
u8 data[];
} __packed;
+static_assert(offsetof(struct glink_msg, data) == sizeof(struct glink_msg_hdr),
+ "struct member likely outside of __struct_group()");
/**
* struct glink_defer_cmd - deferred incoming control message
@@ -48,7 +53,7 @@ struct glink_msg {
struct glink_defer_cmd {
struct list_head node;
- struct glink_msg msg;
+ struct glink_msg_hdr msg;
u8 data[];
};
@@ -455,12 +460,9 @@ static void qcom_glink_intent_req_abort(struct glink_channel *channel)
static int qcom_glink_send_open_req(struct qcom_glink *glink,
struct glink_channel *channel)
{
- struct {
- struct glink_msg msg;
- u8 name[GLINK_NAME_SIZE];
- } __packed req;
+ DEFINE_RAW_FLEX(struct glink_msg, req, data, GLINK_NAME_SIZE);
int name_len = strlen(channel->name) + 1;
- int req_len = ALIGN(sizeof(req.msg) + name_len, 8);
+ int req_len = ALIGN(sizeof(*req) + name_len, 8);
int ret;
unsigned long flags;
@@ -476,12 +478,12 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink,
channel->lcid = ret;
- req.msg.cmd = cpu_to_le16(GLINK_CMD_OPEN);
- req.msg.param1 = cpu_to_le16(channel->lcid);
- req.msg.param2 = cpu_to_le32(name_len);
- strcpy(req.name, channel->name);
+ req->cmd = cpu_to_le16(GLINK_CMD_OPEN);
+ req->param1 = cpu_to_le16(channel->lcid);
+ req->param2 = cpu_to_le32(name_len);
+ strcpy(req->data, channel->name);
- ret = qcom_glink_tx(glink, &req, req_len, NULL, 0, true);
+ ret = qcom_glink_tx(glink, req, req_len, NULL, 0, true);
if (ret)
goto remove_idr;
@@ -826,7 +828,9 @@ static int qcom_glink_rx_defer(struct qcom_glink *glink, size_t extra)
INIT_LIST_HEAD(&dcmd->node);
- qcom_glink_rx_peek(glink, &dcmd->msg, 0, sizeof(dcmd->msg) + extra);
+ qcom_glink_rx_peek(glink,
+ container_of(&dcmd->msg, struct glink_msg, hdr), 0,
+ sizeof(dcmd->msg) + extra);
spin_lock(&glink->rx_lock);
list_add_tail(&dcmd->node, &glink->rx_queue);
@@ -843,7 +847,7 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
struct glink_core_rx_intent *intent;
struct glink_channel *channel;
struct {
- struct glink_msg msg;
+ struct glink_msg_hdr msg;
__le32 chunk_size;
__le32 left_size;
} __packed hdr;
@@ -965,7 +969,7 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink,
};
struct {
- struct glink_msg msg;
+ struct glink_msg_hdr msg;
struct intent_pair intents[];
} __packed * msg;
@@ -1377,7 +1381,7 @@ static int __qcom_glink_send(struct glink_channel *channel,
struct glink_core_rx_intent *tmp;
int iid = 0;
struct {
- struct glink_msg msg;
+ struct glink_msg_hdr msg;
__le32 chunk_size;
__le32 left_size;
} __packed req;
@@ -1685,7 +1689,7 @@ static void qcom_glink_work(struct work_struct *work)
list_del(&dcmd->node);
spin_unlock_irqrestore(&glink->rx_lock, flags);
- msg = &dcmd->msg;
+ msg = container_of(&dcmd->msg, struct glink_msg, hdr);
cmd = le16_to_cpu(msg->cmd);
param1 = le16_to_cpu(msg->param1);
param2 = le32_to_cpu(msg->param2);
--
2.34.1
next reply other threads:[~2024-08-07 15:19 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-07 15:19 Gustavo A. R. Silva [this message]
2024-08-08 18:51 ` [PATCH][next] rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings Kees Cook
2024-08-19 19:45 ` Gustavo A. R. Silva
2024-09-13 8:10 ` Gustavo A. R. Silva
2024-09-13 21:16 ` Bjorn Andersson
-- strict thread matches above, loose matches on Subject: below --
2024-03-25 18:03 Gustavo A. R. Silva
2024-04-29 16:38 ` Kees Cook
2024-08-07 20:43 ` Gustavo A. R. Silva
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=ZrOQa2gew5yadyt3@cute \
--to=gustavoars@kernel.org \
--cc=andersson@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).