[v1] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

[PATCH][next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 9 months ago

-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 an on-stack definition 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 warning:

drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c:39:40: 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>
---

This looks pretty much like the following patch for RTC:

https://lore.kernel.org/linux-hardening/Z9PpPg06OK8ghNvm@kspp/

Thanks

 .../cros_ec_sensors/cros_ec_sensors_core.c    | 27 +++++++------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 7751d6f69b12..760a07f64390 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -34,25 +34,18 @@
 static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
 					     u16 cmd_offset, u16 cmd, u32 *mask)
 {
+	DEFINE_RAW_FLEX(struct cros_ec_command, buf, data,
+			sizeof(struct ec_response_get_cmd_versions));
 	int ret;
-	struct {
-		struct cros_ec_command msg;
-		union {
-			struct ec_params_get_cmd_versions params;
-			struct ec_response_get_cmd_versions resp;
-		};
-	} __packed buf = {
-		.msg = {
-			.command = EC_CMD_GET_CMD_VERSIONS + cmd_offset,
-			.insize = sizeof(struct ec_response_get_cmd_versions),
-			.outsize = sizeof(struct ec_params_get_cmd_versions)
-			},
-		.params = {.cmd = cmd}
-	};
-
-	ret = cros_ec_cmd_xfer_status(ec_dev, &buf.msg);
+
+	buf->command = EC_CMD_GET_CMD_VERSIONS + cmd_offset;
+	buf->insize = sizeof(struct ec_response_get_cmd_versions);
+	buf->outsize = sizeof(struct ec_params_get_cmd_versions);
+	((struct ec_params_get_cmd_versions *)buf->data)->cmd = cmd;
+
+	ret = cros_ec_cmd_xfer_status(ec_dev, buf);
 	if (ret >= 0)
-		*mask = buf.resp.version_mask;
+		*mask = ((struct ec_response_get_cmd_versions *)buf->data)->version_mask;
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH][next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

Posted by Tzung-Bi Shih 9 months ago

On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote:
>  static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
>  					     u16 cmd_offset, u16 cmd, u32 *mask)
>  {
> +	DEFINE_RAW_FLEX(struct cros_ec_command, buf, data,
> +			sizeof(struct ec_response_get_cmd_versions));

max(sizeof(struct ec_params_get_cmd_versions),
    sizeof(struct ec_response_get_cmd_versions))?

Re: [PATCH][next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 9 months ago


On 17/03/25 12:02, Tzung-Bi Shih wrote:
> On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote:
>>   static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
>>   					     u16 cmd_offset, u16 cmd, u32 *mask)
>>   {
>> +	DEFINE_RAW_FLEX(struct cros_ec_command, buf, data,
>> +			sizeof(struct ec_response_get_cmd_versions));
> 
> max(sizeof(struct ec_params_get_cmd_versions),
>      sizeof(struct ec_response_get_cmd_versions))?

I considered that, but DEFINE_RAW_FLEX() complains about it due to the

_Static_assert(__builtin_constant_p(count),				\
		       "onstack flex array members require compile-time const count");

--
Gustavo

Re: [PATCH][next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

Posted by Jonathan Cameron 9 months ago

On Mon, 17 Mar 2025 15:10:38 +1030
"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> On 17/03/25 12:02, Tzung-Bi Shih wrote:
> > On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote:  
> >>   static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
> >>   					     u16 cmd_offset, u16 cmd, u32 *mask)
> >>   {
> >> +	DEFINE_RAW_FLEX(struct cros_ec_command, buf, data,
> >> +			sizeof(struct ec_response_get_cmd_versions));  
> > 
> > max(sizeof(struct ec_params_get_cmd_versions),
> >      sizeof(struct ec_response_get_cmd_versions))?  
> 
> I considered that, but DEFINE_RAW_FLEX() complains about it due to the
> 
> _Static_assert(__builtin_constant_p(count),				\
> 		       "onstack flex array members require compile-time const count");
> 
Maybe add an assert that you indeed have the larger of the two + a comment
on why it matters?

> --
> Gustavo
>

Re: [PATCH][next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

Posted by Tzung-Bi Shih 9 months ago

On Mon, Mar 17, 2025 at 12:04:47PM +0000, Jonathan Cameron wrote:
> On Mon, 17 Mar 2025 15:10:38 +1030
> "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> 
> > On 17/03/25 12:02, Tzung-Bi Shih wrote:
> > > On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote:  
> > >>   static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
> > >>   					     u16 cmd_offset, u16 cmd, u32 *mask)
> > >>   {
> > >> +	DEFINE_RAW_FLEX(struct cros_ec_command, buf, data,
> > >> +			sizeof(struct ec_response_get_cmd_versions));  
> > > 
> > > max(sizeof(struct ec_params_get_cmd_versions),
> > >      sizeof(struct ec_response_get_cmd_versions))?  
> > 
> > I considered that, but DEFINE_RAW_FLEX() complains about it due to the
> > 
> > _Static_assert(__builtin_constant_p(count),				\
> > 		       "onstack flex array members require compile-time const count");
> > 
> Maybe add an assert that you indeed have the larger of the two + a comment
> on why it matters?

Or, is there a way for using compiler-time MAX()?  I failed to find so.

Re: [PATCH][next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

Posted by Kees Cook 9 months ago


On March 17, 2025 6:51:50 PM PDT, Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>On Mon, Mar 17, 2025 at 12:04:47PM +0000, Jonathan Cameron wrote:
>> On Mon, 17 Mar 2025 15:10:38 +1030
>> "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
>> 
>> > On 17/03/25 12:02, Tzung-Bi Shih wrote:
>> > > On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote:  
>> > >>   static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
>> > >>   					     u16 cmd_offset, u16 cmd, u32 *mask)
>> > >>   {
>> > >> +	DEFINE_RAW_FLEX(struct cros_ec_command, buf, data,
>> > >> +			sizeof(struct ec_response_get_cmd_versions));  
>> > > 
>> > > max(sizeof(struct ec_params_get_cmd_versions),
>> > >      sizeof(struct ec_response_get_cmd_versions))?  
>> > 
>> > I considered that, but DEFINE_RAW_FLEX() complains about it due to the
>> > 
>> > _Static_assert(__builtin_constant_p(count),				\
>> > 		       "onstack flex array members require compile-time const count");
>> > 
>> Maybe add an assert that you indeed have the larger of the two + a comment
>> on why it matters?
>
>Or, is there a way for using compiler-time MAX()?  I failed to find so.

Did MAX() not work? I would expect it to do compile time comparison of two sizeof()s. It can do other arithmetic no problem, e.g.:
https://elixir.bootlin.com/linux/v6.13.7/source/lib/vsprintf.c#L1097

-- 
Kees Cook

Re: [PATCH][next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 9 months ago

> Did MAX() not work? I would expect it to do compile time comparison of two sizeof()s. It can do other arithmetic no problem, e.g.:
> https://elixir.bootlin.com/linux/v6.13.7/source/lib/vsprintf.c#L1097

Yep, that does the trick. :)

Thanks
--
Gustavo