-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.
There is currently an object (`msg`) in multiple structures that
contains a flexible structure (`struct glink_msg`), for example:
struct glink_defer_cmd {
...
struct glink_msg msg;
u8 data[];
};
So, in order to avoid ending up with a flexible-array member in the
middle of another structure, we use the `__struct_group()` helper
to separate the flexible array from the rest of the members in the
flexible structure:
struct glink_msg {
__struct_group(glink_msg_hdr, hdr, __packed,
... the rest of members
);
u8 data[];
} __packed;
With the change described above, we now declare objects of the type of
the tagged struct, in this case `struct glink_msg_hdr`, without
embedding flexible arrays in the middle of other structures:
struct glink_defer_cmd {
...
struct glink_msg_hdr msg;
u8 data[];
};
Also, use `container_of()` to retrieve a pointer to the flexible structure.
We also use the `DEFINE_RAW_FLEX()` helper for an in-stack definition of
a flexible structure where the size of 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 <[email protected]>
---
drivers/rpmsg/qcom_glink_native.c | 38 ++++++++++++++++---------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 82d460ff4777..878e3461bce1 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -30,9 +30,12 @@
#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;
@@ -48,7 +51,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 +458,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,10 +476,10 @@ 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);
if (ret)
@@ -826,7 +826,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 +845,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 +967,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 +1379,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 +1687,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
On Mon, Mar 25, 2024 at 12:03:25PM -0600, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> ready to enable it globally.
>
> There is currently an object (`msg`) in multiple structures that
> contains a flexible structure (`struct glink_msg`), for example:
>
> struct glink_defer_cmd {
> ...
>
> struct glink_msg msg;
> u8 data[];
> };
>
> So, in order to avoid ending up with a flexible-array member in the
> middle of another structure, we use the `__struct_group()` helper
> to separate the flexible array from the rest of the members in the
> flexible structure:
>
> struct glink_msg {
> __struct_group(glink_msg_hdr, hdr, __packed,
>
> ... the rest of members
>
> );
> u8 data[];
> } __packed;
>
> With the change described above, we now declare objects of the type of
> the tagged struct, in this case `struct glink_msg_hdr`, without
> embedding flexible arrays in the middle of other structures:
>
> struct glink_defer_cmd {
> ...
>
> struct glink_msg_hdr msg;
> u8 data[];
> };
>
> Also, use `container_of()` to retrieve a pointer to the flexible structure.
>
> We also use the `DEFINE_RAW_FLEX()` helper for an in-stack definition of
> a flexible structure where the size of 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 <[email protected]>
> ---
> drivers/rpmsg/qcom_glink_native.c | 38 ++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 82d460ff4777..878e3461bce1 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -30,9 +30,12 @@
> #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;
>
> @@ -48,7 +51,7 @@ struct glink_msg {
> struct glink_defer_cmd {
> struct list_head node;
>
> - struct glink_msg msg;
> + struct glink_msg_hdr msg;
> u8 data[];
> };
Instead of this change (and the container_of() uses below), I think you
can just simply drop "data" here. I don't see anything using it except
the struct_size()s which can all change their "data" argument to
msg.data. e.g.:
- dcmd = kzalloc(struct_size(dcmd, data, extra), GFP_ATOMIC);
+ dcmd = kzalloc(struct_size(dcmd, msg.data, extra), GFP_ATOMIC);
With those changed, I think this patch becomes more readable.
-Kees
--
Kees Cook