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

Selvin Xavier
2019-08-21T11:11:49Z

On Tue, Aug 20, 2019 at 10:43 PM Doug Ledford <dledford@redhat.com> wrote:
>
> On Mon, 2019-08-19 at 13:42 +0530, Selvin Xavier wrote:
> > 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.
>
> This patch is a lot of lines of churn. What creating an array of sizes
> such that you could use the cmd value as an index into the array. Then
> you'd only need to modify a header file to define the array, and the
> send function to lookup the length of the command based upon the command
> value itself. Far fewer lines of churn, especially if you are possibly
> going to replace this fix in for-next.
Ok. I will work on an optimized solution for this and post it.
Thanks,

>
> --
> Doug Ledford <dledford@redhat.com>
> GPG KeyID: B826A3330E572FDD
> Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD