| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d3336e9d-2b84-4698-a799-b49e3845f38f@embeddedor.com>
Date: Wed, 12 Nov 2025 17:49:05 +0900
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Leon Romanovsky <leon@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>,
Zhu Yanjun <zyjzyj2000@...il.com>, Jason Gunthorpe <jgg@...pe.ca>,
linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] RDMA/rxe: Avoid -Wflex-array-member-not-at-end
warnings
On 11/11/25 23:19, Leon Romanovsky wrote:
> On Tue, Nov 11, 2025 at 09:14:05PM +0900, Gustavo A. R. Silva wrote:
>>
>>
>> On 11/11/25 20:56, Leon Romanovsky wrote:
>>> On Tue, Nov 11, 2025 at 12:35:02PM +0900, 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 new TRAILING_OVERLAP() helper to fix the following warning:
>>>>
>>>> 21 drivers/infiniband/sw/rxe/rxe_verbs.h:271:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>>>
>>>> This helper creates a union between a flexible-array member (FAM) and a
>>>> set of MEMBERS that would otherwise follow it.
>>>>
>>>> This overlays the trailing MEMBER struct ib_sge sge[RXE_MAX_SGE]; onto
>>>> the FAM struct rxe_recv_wqe::dma.sge, while keeping the FAM and the
>>>> start of MEMBER aligned.
>>>>
>>>> The static_assert() ensures this alignment remains, and it's
>>>> intentionally placed inmediately after the related structure --no
>>>> blank line in between.
>>>>
>>>> Lastly, move the conflicting declaration struct rxe_resp_info resp;
>>>> to the end of the corresponding structure.
>>>>
>>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
>>>> ---
>>>> drivers/infiniband/sw/rxe/rxe_verbs.h | 18 +++++++++++-------
>>>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>> index fd48075810dd..6498d61e8956 100644
>>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>> @@ -219,12 +219,6 @@ struct rxe_resp_info {
>>>> u32 rkey;
>>>> u32 length;
>>>> - /* SRQ only */
>>>> - struct {
>>>> - struct rxe_recv_wqe wqe;
>>>> - struct ib_sge sge[RXE_MAX_SGE];
>>>> - } srq_wqe;
>>>> -
>>>> /* Responder resources. It's a circular list where the oldest
>>>> * resource is dropped first.
>>>> */
>>>> @@ -232,7 +226,15 @@ struct rxe_resp_info {
>>>> unsigned int res_head;
>>>> unsigned int res_tail;
>>>> struct resp_res *res;
>>>> +
>>>> + /* SRQ only */
>>>> + /* Must be last as it ends in a flexible-array member. */
>>>> + TRAILING_OVERLAP(struct rxe_recv_wqe, wqe, dma.sge,
>>>> + struct ib_sge sge[RXE_MAX_SGE];
>>>> + ) srq_wqe;
>>>
>>> Will this change be enough?
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>> index fd48075810dd..9ab11421a585 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>> @@ -219,12 +219,6 @@ struct rxe_resp_info {
>>> u32 rkey;
>>> u32 length;
>>> - /* SRQ only */
>>> - struct {
>>> - struct rxe_recv_wqe wqe;
>>> - struct ib_sge sge[RXE_MAX_SGE];
>>> - } srq_wqe;
>>> -
>>> /* Responder resources. It's a circular list where the oldest
>>> * resource is dropped first.
>>> */
>>> @@ -232,6 +226,12 @@ struct rxe_resp_info {
>>> unsigned int res_head;
>>> unsigned int res_tail;
>>> struct resp_res *res;
>>> +
>>> + /* SRQ only */
>>> + struct {
>>> + struct ib_sge sge[RXE_MAX_SGE];
>>> + struct rxe_recv_wqe wqe;
>>> + } srq_wqe;
>>> };
>>
>> The question is if this is really what you want?
>>
>> sge[RXE_MAX_SGE] is of the following type:
>>
>> struct ib_sge {
>> u64 addr;
>> u32 length;
>> u32 lkey;
>> };
>>
>> and struct rxe_recv_wqe::dma.sge[] is of type:
>>
>> struct rxe_sge {
>> __aligned_u64 addr;
>> __u32 length;
>> __u32 lkey;
>> };
>>
>> Both types are basically the same, and the original code looks
>> pretty much like what people do when they want to pre-allocate
>> a number of elements (of the same element type as the flex array)
>> for a flexible-array member.
>>
>> Based on the above, the change you suggest seems a bit suspicious,
>> and I'm not sure that's actually what you want?
>
> You wrote about this error: "warning: structure containing a flexible array
> member is not at the end of another structure".
>
> My suggestion was simply to move that flex array to be the last element
> and save us from the need to have some complex, magic macro in RXE.
Yep, but as I commented above, that doesn't seem to be the right change.
Look at the following couple of lines:
drivers/infiniband/sw/rxe/rxe_resp.c-286- size = sizeof(*wqe) + wqe->dma.num_sge*sizeof(struct rxe_sge);
drivers/infiniband/sw/rxe/rxe_resp.c-287- memcpy(&qp->resp.srq_wqe, wqe, size);
Notice that line 286 is the open-coded arithmetic (struct_size(wqe,
dma.sge, wqe->dma.num_sge) is preferred) to get the number of bytes
to allocate for a flexible structure, in this case struct rxe_recv_wqe,
and its flexible-array member, in this case struct rxe_recv_wqe::dma.sge[].
So, `size` bytes are written in qp->resp.srq_wqe, and the reason this works
seems to be because of the pre-allocation of RXE_MAX_SGE number of elements
for flex array struct rxe_recv_wqe::dma.sge[] given by:
struct {
struct rxe_recv_wqe wqe;
struct ib_sge sge[RXE_MAX_SGE];
} srq_wqe;
So, unless I'm missing something, struct ib_sge sge[RXE_MAX_SGE];
should be aligned with struct rxe_recv_wqe wqe::dma.sge[].
The TRAILING_OVERLAP() macro is also designed to ensure alignment in these
cases (and the static_assert() to preserve it). See this thread:
https://lore.kernel.org/linux-hardening/aLiYrQGdGmaDTtLF@kspp/
Thanks
-Gustavo
Powered by blists - more mailing lists