* [PATCH] RDMA/cxgb4: fix refcounting leak in c4iw_ref_send_wait()
@ 2022-01-24 12:25 Dan Carpenter
2022-01-28 17:16 ` Jason Gunthorpe
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-01-24 12:25 UTC (permalink / raw)
To: Potnuri Bharat Teja, Steve Wise
Cc: Jason Gunthorpe, Doug Ledford, linux-rdma, kernel-janitors
Call c4iw_put_wr_wait() if c4iw_wait_for_reply() fails. This
code uses kobject so the worst impact from this bug is a DoS.
Fixes: 2015f26cfade ("iw_cxgb4: add referencing to wait objects")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
From static analysis. Not tested.
drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 12f33467c672..1dc0e00aba5f 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -297,11 +297,18 @@ static inline int c4iw_ref_send_wait(struct c4iw_rdev *rdev,
qpid);
c4iw_get_wr_wait(wr_waitp);
ret = c4iw_ofld_send(rdev, skb);
- if (ret) {
- c4iw_put_wr_wait(wr_waitp);
- return ret;
- }
- return c4iw_wait_for_reply(rdev, wr_waitp, hwtid, qpid, func);
+ if (ret)
+ goto put_wait;
+
+ ret = c4iw_wait_for_reply(rdev, wr_waitp, hwtid, qpid, func);
+ if (ret)
+ goto put_wait;
+
+ return 0;
+
+put_wait:
+ c4iw_put_wr_wait(wr_waitp);
+ return ret;
}
enum db_state {
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] RDMA/cxgb4: fix refcounting leak in c4iw_ref_send_wait()
2022-01-24 12:25 [PATCH] RDMA/cxgb4: fix refcounting leak in c4iw_ref_send_wait() Dan Carpenter
@ 2022-01-28 17:16 ` Jason Gunthorpe
2022-01-28 19:52 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Jason Gunthorpe @ 2022-01-28 17:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: Potnuri Bharat Teja, Steve Wise, Doug Ledford, linux-rdma,
kernel-janitors
On Mon, Jan 24, 2022 at 03:25:02PM +0300, Dan Carpenter wrote:
> Call c4iw_put_wr_wait() if c4iw_wait_for_reply() fails. This
> code uses kobject so the worst impact from this bug is a DoS.
>
> Fixes: 2015f26cfade ("iw_cxgb4: add referencing to wait objects")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> >From static analysis. Not tested.
>
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
Are you sure?
Looking at the caller alloc_srq_queue() it calls down to
c4iw_ref_send_wait() then immediately exits on failure
The only caller c4iw_create_srq()
ret = alloc_srq_queue(srq, ucontext ? &ucontext->uctx :
&rhp->rdev.uctx, srq->wr_waitp);
if (ret)
goto err_free_skb;
And then
err_free_skb:
kfree_skb(srq->destroy_skb);
err_free_srq_idx:
c4iw_free_srq_idx(&rhp->rdev, srq->idx);
err_free_wr_wait:
c4iw_put_wr_wait(srq->wr_waitp);
So we just double put the thing with this patch
I have no idea how this logic is supposed to work, and clearly
something is buggy in here, but I can't say this is right..
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] RDMA/cxgb4: fix refcounting leak in c4iw_ref_send_wait()
2022-01-28 17:16 ` Jason Gunthorpe
@ 2022-01-28 19:52 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2022-01-28 19:52 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Potnuri Bharat Teja, Steve Wise, Doug Ledford, linux-rdma,
kernel-janitors
On Fri, Jan 28, 2022 at 01:16:36PM -0400, Jason Gunthorpe wrote:
> On Mon, Jan 24, 2022 at 03:25:02PM +0300, Dan Carpenter wrote:
> > Call c4iw_put_wr_wait() if c4iw_wait_for_reply() fails. This
> > code uses kobject so the worst impact from this bug is a DoS.
> >
> > Fixes: 2015f26cfade ("iw_cxgb4: add referencing to wait objects")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > >From static analysis. Not tested.
> >
> > drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 17 ++++++++++++-----
> > 1 file changed, 12 insertions(+), 5 deletions(-)
>
> Are you sure?
>
> Looking at the caller alloc_srq_queue() it calls down to
> c4iw_ref_send_wait() then immediately exits on failure
>
> The only caller c4iw_create_srq()
>
> ret = alloc_srq_queue(srq, ucontext ? &ucontext->uctx :
> &rhp->rdev.uctx, srq->wr_waitp);
> if (ret)
> goto err_free_skb;
>
> And then
>
> err_free_skb:
> kfree_skb(srq->destroy_skb);
> err_free_srq_idx:
> c4iw_free_srq_idx(&rhp->rdev, srq->idx);
> err_free_wr_wait:
> c4iw_put_wr_wait(srq->wr_waitp);
>
> So we just double put the thing with this patch
>
> I have no idea how this logic is supposed to work, and clearly
> something is buggy in here, but I can't say this is right..
Yeah. My patch isn't right. That refcount from my patch is supposed to
be decremented in _c4iw_wake_up(). That function gets called when the
firmware responds in fw6_msg(). So if we cleanup everything and the
firmware sends a delayed response the it leads to a use after free.
Say the firmware never responds, then sure, that leads to a resource
leak. But it's better to have a small memory leak than a use after free.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-01-28 19:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-24 12:25 [PATCH] RDMA/cxgb4: fix refcounting leak in c4iw_ref_send_wait() Dan Carpenter
2022-01-28 17:16 ` Jason Gunthorpe
2022-01-28 19:52 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.