Re: [PATCH][next] rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

On 08/08/24 12:51, Kees Cook wrote:
> On Wed, Aug 07, 2024 at 09:19:07AM -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.
> >
> > So, in order to avoid ending up with a flexible-array member in the
> > middle of multiple other structs, we use the `__struct_group()`
> > helper to create a new tagged `struct glink_msg_hdr`. This structure
> > groups together all the members of the flexible `struct glink_msg`
> > except the flexible array.
> >
> > As a result, the array is effectively separated from the rest of the
> > members without modifying the memory layout of the flexible structure.
> > We then change the type of the middle struct members currently causing
> > trouble from `struct glink_msg` to `struct glink_msg_hdr`.
> >
> > We also want to ensure that when new members need to be added to the
> > flexible structure, they are always included within the newly created
> > tagged struct. For this, we use `static_assert()`. This ensures that the
> > memory layout for both the flexible structure and the new tagged struct
> > is the same after any changes.
> >
> > This approach avoids having to implement `struct glink_msg_hdr` as a
> > completely separate structure, thus preventing having to maintain two
> > independent but basically identical structures, closing the door to
> > potential bugs in the future.
> >
> > We also use `container_of()` whenever we need to retrieve a pointer to
> > the flexible structure, through which we can access the flexible-array
> > member, if necessary.
> >
> > Additionally, we use the `DEFINE_RAW_FLEX()` helper for an on-stack
> > definition of a flexible structure where the size for 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 <gustavoars@xxxxxxxxxx>
>
> Looks correct to me. As a separate change, I wonder if the strcpy()
> should be replaced with strscpy_pad(), but I think it's all okay as-is,
> since channel->name seems to be set from another fixed-size array that
> is the same size.

Yes, I noticed the same after sending the patch. :p

> Reviewed-by: Kees Cook <kees@xxxxxxxxxx>

Thanks!
--
Gustavo