rtc: Avoid a couple of -Wflex-array-member-not-at-end warnings
Subsystems (2)
RFC 1/1 rtc: Avoid a couple of -Wflex-array-member-not-at-end warnings
Hi all,
As part of the efforts to globally enable -Wflex-array-member-not-at-end, I'm currently trying to fix the following warnings:
drivers/rtc/rtc-cros-ec.c:62:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] drivers/rtc/rtc-cros-ec.c:40:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
The issue is that struct cros_ec_command is a flexible structure (which means that it contains a flexible-array member), and there is an object of this type (msg) declared within another structure but at the end.
It seems that the following patch would suffice, as long as the flex-array member in struct cros_ec_command is not expected to be accessed and overlap with struct ec_response_rtc data in a "controlled manner":
drivers/rtc/rtc-cros-ec.c +2 -2
@@ -37,8 +37,8 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, {int ret;struct { -struct cros_ec_command msg;struct ec_response_rtc data; +struct cros_ec_command msg; } __packed msg;memset (&msg,0 ,sizeof (msg));
@@ -59,8 +59,8 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, {int ret;struct { -struct cros_ec_command msg;struct ec_response_rtc data; +struct cros_ec_command msg; } __packed msg;memset (&msg,0 ,sizeof (msg));
Otherwise, we probably need to use struct_group_tagged() as follows:
drivers/rtc/rtc-cros-ec.c +10 -4
@@ -37,7 +37,7 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, {int ret;struct { -struct cros_ec_command msg; +struct cros_ec_command_hdr msg;struct ec_response_rtc data; } __packed msg;
@@ -45,7 +45,10 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, msg.msg.command = command; msg.msg.insize =sizeof (msg.data); - ret =cros_ec_cmd_xfer_status (cros_ec, &msg.msg); + ret =cros_ec_cmd_xfer_status (cros_ec, +container_of (&msg.msg, + struct cros_ec_command, + __hdr));if (ret <0 )return ret;
@@ -59,7 +62,7 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, {int ret;struct { -struct cros_ec_command msg; +struct cros_ec_command_hdr msg;struct ec_response_rtc data; } __packed msg;
@@ -68,7 +71,10 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, msg.msg.outsize =sizeof (msg.data); msg.data.time = param; - ret =cros_ec_cmd_xfer_status (cros_ec, &msg.msg); + ret =cros_ec_cmd_xfer_status (cros_ec, +container_of (&msg.msg, + struct cros_ec_command, + __hdr));if (ret <0 )return ret;return 0 ;
include/linux/platform_data/cros_ec_proto.h +7 -5
@@ -80,11 +80,13 @@ enum {* @data: Where to put the incoming data from EC and outgoing data to EC. */ struct cros_ec_command { -uint32_t version; -uint32_t command; -uint32_t outsize; -uint32_t insize; -uint32_t result; + struct_group_tagged(cros_ec_command_hdr, __hdr, + uint32_t version; +uint32_t command; +uint32_t outsize; +uint32_t insize; +uint32_t result; + );uint8_t data[]; };
What do you think?
Thanks!
--
Gustavo
Quoted 11 lines · Gustavo A. R. Silva
Hi all,
As part of the efforts to globally enable -Wflex-array-member-not-at-end, I'm currently trying to fix the following warnings:
drivers/rtc/rtc-cros-ec.c:62:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]drivers/rtc/rtc-cros-ec.c:40:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]The issue is that
struct cros_ec_commandis a flexible structure (which means that it contains a flexible-array member), and there is an object of this type (msg) declared within another structure but at the end.
^ not
Quoted 19 lines
It seems that the following patch would suffice, as long as the flex-array member in
struct cros_ec_commandis not expected to be accessed and overlap withstruct ec_response_rtc datain a "controlled manner":drivers/rtc/rtc-cros-ec.c +1
-1@@ -37,8 +37,8 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, {int ret;struct { -struct cros_ec_command msg;struct ec_response_rtc data; +struct cros_ec_command msg; } __packed msg;memset (&msg,0 ,sizeof (msg));
drivers/rtc/rtc-cros-ec.c static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, · line 66
Quoted 9 lines
drivers/rtc/rtc-cros-ec.c +1
-1@@ -59,8 +59,8 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, {int ret;struct { -struct cros_ec_command msg;struct ec_response_rtc data; +struct cros_ec_command msg; } __packed msg;memset (&msg,0 ,sizeof (msg));
This doesn't work. The header (i.e. struct cros_ec_command) is expected to allocate right before the payload (e.g. struct ec_response_rtc)[1][2].
[1]: https://elixir.bootlin.com/linux/v6.14-rc1/source/drivers/platform/chrome/cros_ec_proto.c#L82
[2]: https://elixir.bootlin.com/linux/v6.14-rc1/source/drivers/platform/chrome/cros_ec_i2c.c#L166
Quoted 1 line
Otherwise, we probably need to use
struct_group_tagged()as follows:
I'm not a big fan of the solution. How about using something similar to:
struct cros_ec_command *msg = kzalloc(sizeof(*msg) +
sizeof(struct ec_response_rtc), ...);
Quoted 6 lines
I'm not a big fan of the solution. How about using something similar to:
struct
cros_ec_command*msg = kzalloc(sizeof(*msg) +sizeof(struct ec_response_rtc), ...);
Thanks for the feedback. :) We can actually use DEFINE_RAW_FLEX():
https://lore.kernel.org/linux-hardening/Z9N8BsVJF-s6Hcvd@kspp/
--
Gustavo