* [PATCH][next] w1: Avoid -Wflex-array-member-not-at-end warnings
@ 2025-03-27 16:59 Gustavo A. R. Silva
2025-04-07 19:09 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2025-03-27 16:59 UTC (permalink / raw)
To: Krzysztof Kozlowski; +Cc: linux-kernel, Gustavo A. R. Silva, linux-hardening
-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.
Use the `DEFINE_RAW_FLEX()` helper for on-stack definitions of
a flexible structure where the size of the flexible-array member
is known at compile-time, and refactor the rest of the code,
accordingly.
So, with these changes, fix the following warnings:
drivers/w1/w1_netlink.c:198:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/w1/w1_netlink.c:219:31: 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/w1/w1_netlink.c | 41 +++++++++++++++++++----------------------
1 file changed, 19 insertions(+), 22 deletions(-)
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index 691978cddab7..845d66ab7e89 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -194,16 +194,16 @@ static void w1_netlink_queue_status(struct w1_cb_block *block,
static void w1_netlink_send_error(struct cn_msg *cn, struct w1_netlink_msg *msg,
int portid, int error)
{
- struct {
- struct cn_msg cn;
- struct w1_netlink_msg msg;
- } packet;
- memcpy(&packet.cn, cn, sizeof(packet.cn));
- memcpy(&packet.msg, msg, sizeof(packet.msg));
- packet.cn.len = sizeof(packet.msg);
- packet.msg.len = 0;
- packet.msg.status = (u8)-error;
- cn_netlink_send(&packet.cn, portid, 0, GFP_KERNEL);
+ DEFINE_RAW_FLEX(struct cn_msg, packet, data,
+ sizeof(struct w1_netlink_msg));
+ struct w1_netlink_msg *pkt_msg = (struct w1_netlink_msg *)packet->data;
+
+ memcpy(packet, cn, sizeof(*packet));
+ memcpy(pkt_msg, msg, sizeof(*pkt_msg));
+ packet->len = sizeof(*pkt_msg);
+ pkt_msg->len = 0;
+ pkt_msg->status = (u8)-error;
+ cn_netlink_send(packet, portid, 0, GFP_KERNEL);
}
/**
@@ -215,22 +215,19 @@ static void w1_netlink_send_error(struct cn_msg *cn, struct w1_netlink_msg *msg,
*/
void w1_netlink_send(struct w1_master *dev, struct w1_netlink_msg *msg)
{
- struct {
- struct cn_msg cn;
- struct w1_netlink_msg msg;
- } packet;
- memset(&packet, 0, sizeof(packet));
+ DEFINE_RAW_FLEX(struct cn_msg, packet, data,
+ sizeof(struct w1_netlink_msg));
- packet.cn.id.idx = CN_W1_IDX;
- packet.cn.id.val = CN_W1_VAL;
+ packet->id.idx = CN_W1_IDX;
+ packet->id.val = CN_W1_VAL;
- packet.cn.seq = dev->seq++;
- packet.cn.len = sizeof(*msg);
+ packet->seq = dev->seq++;
+ packet->len = sizeof(*msg);
- memcpy(&packet.msg, msg, sizeof(*msg));
- packet.msg.len = 0;
+ memcpy(packet, msg, sizeof(*msg));
+ ((struct w1_netlink_msg *)packet->data)->len = 0;
- cn_netlink_send(&packet.cn, 0, 0, GFP_KERNEL);
+ cn_netlink_send(packet, 0, 0, GFP_KERNEL);
}
static void w1_send_slave(struct w1_master *dev, u64 rn)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH][next] w1: Avoid -Wflex-array-member-not-at-end warnings
2025-03-27 16:59 [PATCH][next] w1: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
@ 2025-04-07 19:09 ` Kees Cook
2025-04-07 19:19 ` Gustavo A. R. Silva
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2025-04-07 19:09 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: Krzysztof Kozlowski, linux-kernel, linux-hardening
On Thu, Mar 27, 2025 at 10:59:04AM -0600, 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.
>
> Use the `DEFINE_RAW_FLEX()` helper for on-stack definitions of
> a flexible structure where the size of the flexible-array member
> is known at compile-time, and refactor the rest of the code,
> accordingly.
>
> So, with these changes, fix the following warnings:
>
> drivers/w1/w1_netlink.c:198:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> drivers/w1/w1_netlink.c:219:31: 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/w1/w1_netlink.c | 41 +++++++++++++++++++----------------------
> 1 file changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
> index 691978cddab7..845d66ab7e89 100644
> --- a/drivers/w1/w1_netlink.c
> +++ b/drivers/w1/w1_netlink.c
> @@ -194,16 +194,16 @@ static void w1_netlink_queue_status(struct w1_cb_block *block,
> static void w1_netlink_send_error(struct cn_msg *cn, struct w1_netlink_msg *msg,
> int portid, int error)
> {
> - struct {
> - struct cn_msg cn;
> - struct w1_netlink_msg msg;
> - } packet;
> - memcpy(&packet.cn, cn, sizeof(packet.cn));
> - memcpy(&packet.msg, msg, sizeof(packet.msg));
> - packet.cn.len = sizeof(packet.msg);
> - packet.msg.len = 0;
> - packet.msg.status = (u8)-error;
> - cn_netlink_send(&packet.cn, portid, 0, GFP_KERNEL);
> + DEFINE_RAW_FLEX(struct cn_msg, packet, data,
> + sizeof(struct w1_netlink_msg));
> + struct w1_netlink_msg *pkt_msg = (struct w1_netlink_msg *)packet->data;
I'm starting to think we need a helper for "DEFINE_RAW_FLEX with a
trailing structure" for these. :)
Anyway, conversion looks good... structs are packed, so alignment issues
are unchanged.
> +
> + memcpy(packet, cn, sizeof(*packet));
> + memcpy(pkt_msg, msg, sizeof(*pkt_msg));
These could just be:
*packet = *cn;
*pkg_msg = *msg;
But that was always true. The memcpy() style is retained. But it would
catch type mismatches (like is accidentally introduced below).
> + packet->len = sizeof(*pkt_msg);
> + pkt_msg->len = 0;
> + pkt_msg->status = (u8)-error;
> + cn_netlink_send(packet, portid, 0, GFP_KERNEL);
> }
>
> /**
> @@ -215,22 +215,19 @@ static void w1_netlink_send_error(struct cn_msg *cn, struct w1_netlink_msg *msg,
> */
> void w1_netlink_send(struct w1_master *dev, struct w1_netlink_msg *msg)
> {
> - struct {
> - struct cn_msg cn;
> - struct w1_netlink_msg msg;
> - } packet;
> - memset(&packet, 0, sizeof(packet));
> + DEFINE_RAW_FLEX(struct cn_msg, packet, data,
> + sizeof(struct w1_netlink_msg));
>
> - packet.cn.id.idx = CN_W1_IDX;
> - packet.cn.id.val = CN_W1_VAL;
> + packet->id.idx = CN_W1_IDX;
> + packet->id.val = CN_W1_VAL;
>
> - packet.cn.seq = dev->seq++;
> - packet.cn.len = sizeof(*msg);
> + packet->seq = dev->seq++;
> + packet->len = sizeof(*msg);
>
> - memcpy(&packet.msg, msg, sizeof(*msg));
> - packet.msg.len = 0;
> + memcpy(packet, msg, sizeof(*msg));
This memcpy() is wrong. It should be targeting packet->data.
> + ((struct w1_netlink_msg *)packet->data)->len = 0;
And since you need it again here, I'd recommend defining a struct
w1_netlink_msg pointer similar to the first hunk.
-Kees
>
> - cn_netlink_send(&packet.cn, 0, 0, GFP_KERNEL);
> + cn_netlink_send(packet, 0, 0, GFP_KERNEL);
> }
>
> static void w1_send_slave(struct w1_master *dev, u64 rn)
> --
> 2.43.0
>
--
Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH][next] w1: Avoid -Wflex-array-member-not-at-end warnings
2025-04-07 19:09 ` Kees Cook
@ 2025-04-07 19:19 ` Gustavo A. R. Silva
0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2025-04-07 19:19 UTC (permalink / raw)
To: Kees Cook, Gustavo A. R. Silva
Cc: Krzysztof Kozlowski, linux-kernel, linux-hardening
On 07/04/25 13:09, Kees Cook wrote:
> On Thu, Mar 27, 2025 at 10:59:04AM -0600, 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.
>>
>> Use the `DEFINE_RAW_FLEX()` helper for on-stack definitions of
>> a flexible structure where the size of the flexible-array member
>> is known at compile-time, and refactor the rest of the code,
>> accordingly.
>>
>> So, with these changes, fix the following warnings:
>>
>> drivers/w1/w1_netlink.c:198:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>> drivers/w1/w1_netlink.c:219:31: 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/w1/w1_netlink.c | 41 +++++++++++++++++++----------------------
>> 1 file changed, 19 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
>> index 691978cddab7..845d66ab7e89 100644
>> --- a/drivers/w1/w1_netlink.c
>> +++ b/drivers/w1/w1_netlink.c
>> @@ -194,16 +194,16 @@ static void w1_netlink_queue_status(struct w1_cb_block *block,
>> static void w1_netlink_send_error(struct cn_msg *cn, struct w1_netlink_msg *msg,
>> int portid, int error)
>> {
>> - struct {
>> - struct cn_msg cn;
>> - struct w1_netlink_msg msg;
>> - } packet;
>> - memcpy(&packet.cn, cn, sizeof(packet.cn));
>> - memcpy(&packet.msg, msg, sizeof(packet.msg));
>> - packet.cn.len = sizeof(packet.msg);
>> - packet.msg.len = 0;
>> - packet.msg.status = (u8)-error;
>> - cn_netlink_send(&packet.cn, portid, 0, GFP_KERNEL);
>> + DEFINE_RAW_FLEX(struct cn_msg, packet, data,
>> + sizeof(struct w1_netlink_msg));
>> + struct w1_netlink_msg *pkt_msg = (struct w1_netlink_msg *)packet->data;
>
> I'm starting to think we need a helper for "DEFINE_RAW_FLEX with a
> trailing structure" for these. :)
>
> Anyway, conversion looks good... structs are packed, so alignment issues
> are unchanged.
>
>> +
>> + memcpy(packet, cn, sizeof(*packet));
>> + memcpy(pkt_msg, msg, sizeof(*pkt_msg));
>
> These could just be:
>
> *packet = *cn;
> *pkg_msg = *msg;
>
> But that was always true. The memcpy() style is retained. But it would
> catch type mismatches (like is accidentally introduced below).
>
>> + packet->len = sizeof(*pkt_msg);
>> + pkt_msg->len = 0;
>> + pkt_msg->status = (u8)-error;
>> + cn_netlink_send(packet, portid, 0, GFP_KERNEL);
>> }
>>
>> /**
>> @@ -215,22 +215,19 @@ static void w1_netlink_send_error(struct cn_msg *cn, struct w1_netlink_msg *msg,
>> */
>> void w1_netlink_send(struct w1_master *dev, struct w1_netlink_msg *msg)
>> {
>> - struct {
>> - struct cn_msg cn;
>> - struct w1_netlink_msg msg;
>> - } packet;
>> - memset(&packet, 0, sizeof(packet));
>> + DEFINE_RAW_FLEX(struct cn_msg, packet, data,
>> + sizeof(struct w1_netlink_msg));
>>
>> - packet.cn.id.idx = CN_W1_IDX;
>> - packet.cn.id.val = CN_W1_VAL;
>> + packet->id.idx = CN_W1_IDX;
>> + packet->id.val = CN_W1_VAL;
>>
>> - packet.cn.seq = dev->seq++;
>> - packet.cn.len = sizeof(*msg);
>> + packet->seq = dev->seq++;
>> + packet->len = sizeof(*msg);
>>
>> - memcpy(&packet.msg, msg, sizeof(*msg));
>> - packet.msg.len = 0;
>> + memcpy(packet, msg, sizeof(*msg));
>
> This memcpy() is wrong. It should be targeting packet->data.
Ah yes! Other similar instances of this (in other subsystems) use `msg`
as an alias for the flexible structure, so it seems I instinctively
thought `packet == msg` for a moment. Thanks for catching this! :)
>
>> + ((struct w1_netlink_msg *)packet->data)->len = 0;
>
> And since you need it again here, I'd recommend defining a struct
> w1_netlink_msg pointer similar to the first hunk.
Right!
Thanks
--
Gustavo
>
> -Kees
>
>>
>> - cn_netlink_send(&packet.cn, 0, 0, GFP_KERNEL);
>> + cn_netlink_send(packet, 0, 0, GFP_KERNEL);
>> }
>>
>> static void w1_send_slave(struct w1_master *dev, u64 rn)
>> --
>> 2.43.0
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-07 19:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-27 16:59 [PATCH][next] w1: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
2025-04-07 19:09 ` Kees Cook
2025-04-07 19:19 ` Gustavo A. R. Silva
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.