| Message ID | 20201230024653.1516495-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA/ocrdma: fix use after free in ocrdma_dealloc_ucontext_pd() | expand |
On Tue, Dec 29, 2020 at 06:46:53PM -0800, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > In ocrdma_dealloc_ucontext_pd() uctx->cntxt_pd is assigned to > the variable pd and then after uctx->cntxt_pd is freed, the > variable pd is passed to function _ocrdma_dealloc_pd() which > dereferences pd directly or through its call to > ocrdma_mbx_dealloc_pd(). > > Reorder the free using the variable pd. > > Fixes: 21a428a019c9 ("RDMA: Handle PD allocations by IB/core") > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Thanks, Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
On Tue, Dec 29, 2020 at 06:46:53PM -0800, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > In ocrdma_dealloc_ucontext_pd() uctx->cntxt_pd is assigned to > the variable pd and then after uctx->cntxt_pd is freed, the > variable pd is passed to function _ocrdma_dealloc_pd() which > dereferences pd directly or through its call to > ocrdma_mbx_dealloc_pd(). > > Reorder the free using the variable pd. > > Fixes: 21a428a019c9 ("RDMA: Handle PD allocations by IB/core") > Signed-off-by: Tom Rix <trix@redhat.com> > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied to for-rc Is anyone testing ocrdma? Just doing the pyverbs rdma tests with kasn turned on would have instantly caught this, and the change is nearly a year old. Is ocrdma obsolete enough we can delete the driver? Thanks, Jason
On 1/7/21 12:41 PM, Jason Gunthorpe wrote: > On Tue, Dec 29, 2020 at 06:46:53PM -0800, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> In ocrdma_dealloc_ucontext_pd() uctx->cntxt_pd is assigned to >> the variable pd and then after uctx->cntxt_pd is freed, the >> variable pd is passed to function _ocrdma_dealloc_pd() which >> dereferences pd directly or through its call to >> ocrdma_mbx_dealloc_pd(). >> >> Reorder the free using the variable pd. >> >> Fixes: 21a428a019c9 ("RDMA: Handle PD allocations by IB/core") >> Signed-off-by: Tom Rix <trix@redhat.com> >> drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > Applied to for-rc > > Is anyone testing ocrdma? Just doing the pyverbs rdma tests with kasn > turned on would have instantly caught this, and the change is nearly a > year old. > > Is ocrdma obsolete enough we can delete the driver? I am not an authority on ocrdma, i am fixing treewide, the problems clang static analysis flags. Tom > > Thanks, > Jason >
On Fri, Jan 8, 2021 at 3:13 AM Tom Rix <trix@redhat.com> wrote: > > > On 1/7/21 12:41 PM, Jason Gunthorpe wrote: > > On Tue, Dec 29, 2020 at 06:46:53PM -0800, trix@redhat.com wrote: > >> From: Tom Rix <trix@redhat.com> > >> > >> In ocrdma_dealloc_ucontext_pd() uctx->cntxt_pd is assigned to > >> the variable pd and then after uctx->cntxt_pd is freed, the > >> variable pd is passed to function _ocrdma_dealloc_pd() which > >> dereferences pd directly or through its call to > >> ocrdma_mbx_dealloc_pd(). > >> > >> Reorder the free using the variable pd. > >> > >> Fixes: 21a428a019c9 ("RDMA: Handle PD allocations by IB/core") > >> Signed-off-by: Tom Rix <trix@redhat.com> > >> drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Applied to for-rc > > > > Is anyone testing ocrdma? Just doing the pyverbs rdma tests with kasn > > turned on would have instantly caught this, and the change is nearly a > > year old. > > > > Is ocrdma obsolete enough we can delete the driver? Broadcom is not doing any active development/testing with ocrdma now. I am checking with other teams to see if this can be deleted completely. Will get back asap. Thanks, Selvin > > I am not an authority on ocrdma, i am fixing treewide, the problems clang static analysis flags. > > Tom > > > > > Thanks, > > Jason > > >
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c index bc98bd950d99..3acb5c10b155 100644 --- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c +++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c @@ -434,9 +434,9 @@ static void ocrdma_dealloc_ucontext_pd(struct ocrdma_ucontext *uctx) pr_err("%s(%d) Freeing in use pdid=0x%x.\n", __func__, dev->id, pd->id); } - kfree(uctx->cntxt_pd); uctx->cntxt_pd = NULL; _ocrdma_dealloc_pd(dev, pd); + kfree(pd); } static struct ocrdma_pd *ocrdma_get_ucontext_pd(struct ocrdma_ucontext *uctx)