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

Selvin Xavier
2019-08-19T08:13:05Z

On Fri, Aug 16, 2019 at 5:30 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> 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.
>
Ok. Will make this change. But this looks like a for-next item.. right?
If so, can you take this patch as is for RC? I will post the change
mentioned for for-next separately.

> > 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.
>
Will check internally and see if it is possible.
> Jason