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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251224065107.GD11869@unreal>
Date: Wed, 24 Dec 2025 08:51:07 +0200
From: Leon Romanovsky <leon@...nel.org>
To: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Cc: Greg Sword <gregsword0@...il.com>, Zhu Yanjun <yanjun.zhu@...ux.dev>,
	zyjzyj2000@...il.com, jgg@...pe.ca, linux-rdma@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>
Subject: Re: [PATCH v3 1/1] RDMA/rxe: Avoid -Wflex-array-member-not-at-end
 warnings

On Wed, Dec 24, 2025 at 02:26:22AM +0900, Gustavo A. R. Silva wrote:
> 
> 
> On 12/24/25 02:19, Greg Sword wrote:
> > On Wed, Dec 24, 2025 at 12:59 AM Gustavo A. R. Silva
> > <gustavo@...eddedor.com> wrote:
> > > 
> > > 
> > > 
> > > On 12/24/25 01:38, Greg Sword wrote:
> > > > On Tue, Dec 23, 2025 at 5:35 PM Gustavo A. R. Silva
> > > > <gustavo@...eddedor.com> wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 12/23/25 13:41, Zhu Yanjun wrote:
> > > > > > From: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> > > > > > 
> > > > > > -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.
> > > > > > 
> > > > > > Reviewed-by: Zhu Yanjun <yanjun.zhu@...ux.dev>
> > > > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
> > > > > 
> > > > > NACK.
> > > > 
> > > > Just a small reminder about community conventions: reviewers can NACK
> > > > a patch, but authors generally should not NACK their own patches.
> > > 
> > > It's obvious that you don't understand what's going on here.
> > 
> > I’ve read through the full discussion and understand how this evolved.
> > Based on that, I believe there may be a misunderstanding on your side.
> 
> Okay, thanks for your contribution then.

Gustavo,

A more fundamental issue than patch granularity is simply treating others
with respect, even when you disagree with them.

Zhu cares about RXE, and he respected your work by trying to address your
problem, not his own. RXE functions correctly without your patch, and this
hardening effort does not apply to RXE at all.

So please remember that both of you want the same thing, and let's keep the
discussion focused on the technical issues.

Thanks

> 
> -Gustavo
> 
> > 
> > > 
> > > > > I didn't write this patch.
> > > 
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > This line should've given you a clue.
> > > 
> > > -Gustavo
> > > 
> > > > > 
> > > > > Please, don't ever submit modified patches on my behalf.
> > > > > 
> > > > > > ---
> > > > > > V2->V3: Replace struct ib_sge with struct rxe_sge
> > > > > 
> > > > > Patch granularity is a fundamental thing. Changes addressing different
> > > > > issues should not be mixed together. Previously existing issues (if any)
> > > > > must be addressed in separate patches.
> > > > > 
> > > > > -Gustavo
> > > > > 
> > > > > > ---
> > > > > >     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..3ffd7be8e7b1 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 rxe_sge          sge[RXE_MAX_SGE];
> > > > > > +     ) srq_wqe;
> > > > > >     };
> > > > > > +static_assert(offsetof(struct rxe_resp_info, srq_wqe.wqe.dma.sge) ==
> > > > > > +           offsetof(struct rxe_resp_info, srq_wqe.sge));
> > > > > > 
> > > > > >     struct rxe_qp {
> > > > > >         struct ib_qp            ibqp;
> > > > > > @@ -269,7 +271,6 @@ struct rxe_qp {
> > > > > > 
> > > > > >         struct rxe_req_info     req;
> > > > > >         struct rxe_comp_info    comp;
> > > > > > -     struct rxe_resp_info    resp;
> > > > > > 
> > > > > >         atomic_t                ssn;
> > > > > >         atomic_t                skb_out;
> > > > > > @@ -289,6 +290,9 @@ struct rxe_qp {
> > > > > >         spinlock_t              state_lock; /* guard requester and completer */
> > > > > > 
> > > > > >         struct execute_work     cleanup_work;
> > > > > > +
> > > > > > +     /* Must be last as it ends in a flexible-array member. */
> > > > > > +     struct rxe_resp_info    resp;
> > > > > >     };
> > > > > > 
> > > > > >     enum {
> > > > > 
> > > > > 
> > > 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ