Re: [PATCH for-rc] RDMA/bnxt_re: Fix stack-out-of-bounds in bnxt_qplib_rcfw_send_message

From: Jason Gunthorpe <jgg@ziepe.ca>
To: Selvin Xavier <selvin.xavier@broadcom.com>
Cc: Doug Ledford <dledford@redhat.com>, linux-rdma@vger.kernel.org
Subject: Re: [PATCH for-rc] RDMA/bnxt_re: Fix stack-out-of-bounds in bnxt_qplib_rcfw_send_message
Date: Fri, 16 Aug 2019 09:00:23 -0300	[thread overview]
Message-ID: <20190816120023.GA5398@ziepe.ca> (raw)
In-Reply-To: <CA+sbYW3t2=bCpYjkuNQeT3LSFcL9n9=awHYpSrB6VZBna4dWhg@mail.gmail.com>

On Fri, Aug 16, 2019 at 10:22:25AM +0530, Selvin Xavier wrote:
> On Thu, Aug 15, 2019 at 6:37 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Thu, Aug 15, 2019 at 04:44:37AM -0700, Selvin Xavier wrote:
> > > @@ -583,7 +584,7 @@ int bnxt_qplib_create_srq(struct bnxt_qplib_res *res,
> > >       req.eventq_id = cpu_to_le16(srq->eventq_hw_ring_id);
> > >
> > >       rc = bnxt_qplib_rcfw_send_message(rcfw, (void *)&req,
> > > -                                       (void *)&resp, NULL, 0);
> > > +                                       (void *)&resp, sizeof(req), NULL, 0);
> >
> > I really don't like seeing casts to void * in code. Why can't you
> > properly embed the header in the structs??
> Is your objection only in casting to void * or you dont like any
> casting here? 

Explicit cast to void to erase the type is a particularly bad habit
that I don't like to see.

You'd be better to make the send_message accept void * and the cast
inside to the header.

> These structures are directly copied from some auto-generated files.

Fix the auto generator? With a proper header struct is the best way to
code this pattern.

Jason