| Message ID | Z9lE6IVDeC5lnChN@kspp (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning | expand |
On Tue, Mar 18, 2025 at 08:33:20PM +1030, 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 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> > --- > Changes in v2: > - Use MAX() to calculate the number of bytes for the flex array. (Kees) > > v1: > - Link: https://lore.kernel.org/linux-hardening/Z9dy43vUUh4goi-Q@kspp/ > > .../cros_ec_sensors/cros_ec_sensors_core.c | 28 ++++++++----------- > 1 file changed, 11 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..40d5b10c74e0 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,19 @@ > 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, > + MAX(sizeof(struct ec_response_get_cmd_versions), > + sizeof(struct ec_params_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; > } Since "params" is used twice, I'd say do like the other patch and assign it to a new variable: struct ec_response_get_cmd_versions *params = buf->data; ... params->cmd = cmd; ... *mask = params->version_mask; > > -- > 2.43.0 >
On Tue, Mar 18, 2025 at 08:42:41AM -0700, Kees Cook wrote: > > + ((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; > > } > > Since "params" is used twice, I'd say do like the other patch and assign > it to a new variable: > > struct ec_response_get_cmd_versions *params = buf->data; > ... > params->cmd = cmd; > ... > *mask = params->version_mask; Not a pushback for introducing new variables. FWIW, it uses "param" and "response" only once.
On Tue, Mar 18, 2025 at 08:33:20PM +1030, 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 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> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
On Wed, 19 Mar 2025 09:13:45 +0000 Tzung-Bi Shih <tzungbi@kernel.org> wrote: > On Tue, Mar 18, 2025 at 08:42:41AM -0700, Kees Cook wrote: > > > + ((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; > > > } > > > > Since "params" is used twice, I'd say do like the other patch and assign > > it to a new variable: > > > > struct ec_response_get_cmd_versions *params = buf->data; > > ... > > params->cmd = cmd; > > ... > > *mask = params->version_mask; > > Not a pushback for introducing new variables. FWIW, it uses "param" and > "response" only once. Given that reply I'm assuming Kees is fine with this. Applied. Thanks, Jonathan
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..40d5b10c74e0 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,19 @@ 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, + MAX(sizeof(struct ec_response_get_cmd_versions), + sizeof(struct ec_params_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; }
-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> --- Changes in v2: - Use MAX() to calculate the number of bytes for the flex array. (Kees) v1: - Link: https://lore.kernel.org/linux-hardening/Z9dy43vUUh4goi-Q@kspp/ .../cros_ec_sensors/cros_ec_sensors_core.c | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-)