From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
To: Kees Cook <kees@kernel.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
Mathieu Poirier <mathieu.poirier@linaro.org>,
linux-arm-msm@vger.kernel.org, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH][next] rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings
Date: Mon, 19 Aug 2024 13:45:43 -0600 [thread overview]
Message-ID: <574744fe-4df3-405c-b4fc-7209d48c1dff@embeddedor.com> (raw)
In-Reply-To: <202408081146.09AA68D69@keescook>
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@kernel.org>
>
> 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@kernel.org>
>
Thanks!
--
Gustavo
next prev parent reply other threads:[~2024-08-19 19:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-07 15:19 [PATCH][next] rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
2024-08-08 18:51 ` Kees Cook
2024-08-19 19:45 ` Gustavo A. R. Silva [this message]
2024-09-13 8:10 ` Gustavo A. R. Silva
2024-09-13 21:16 ` Bjorn Andersson
-- strict thread matches above, loose matches on Subject: below --
2024-03-25 18:03 Gustavo A. R. Silva
2024-04-29 16:38 ` Kees Cook
2024-08-07 20:43 ` Gustavo A. R. Silva
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=574744fe-4df3-405c-b4fc-7209d48c1dff@embeddedor.com \
--to=gustavo@embeddedor.com \
--cc=andersson@kernel.org \
--cc=gustavoars@kernel.org \
--cc=kees@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).