diff mbox

[1/1] RDMA/uverbs: Initialize cq_context appropriately

Message ID 1501565315-3183-1-git-send-email-bharat@chelsio.com (mailing list archive)
State Accepted
Headers show

Commit Message

Potnuri Bharat Teja Aug. 1, 2017, 5:28 a.m. UTC
Initializing cq_context with ev_queue in create_cq(), leads to NULL pointer
dereference in ib_uverbs_comp_handler(), if application doesnot use completion
channel. This patch fixes the cq_context initialization.

Fixes: 1e7710f3f65 ("IB/core: Change completion channel to use the reworked")
Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
---
 drivers/infiniband/core/uverbs_cmd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Matan Barak Aug. 6, 2017, 8:08 a.m. UTC | #1
On Tue, Aug 1, 2017 at 8:28 AM, Potnuri Bharat Teja <bharat@chelsio.com> wrote:
> Initializing cq_context with ev_queue in create_cq(), leads to NULL pointer
> dereference in ib_uverbs_comp_handler(), if application doesnot use completion
> channel. This patch fixes the cq_context initialization.
>
> Fixes: 1e7710f3f65 ("IB/core: Change completion channel to use the reworked")
> Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> ---
>  drivers/infiniband/core/uverbs_cmd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 2c98533a0203..8c829522d8c4 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -1015,7 +1015,8 @@ static struct ib_ucq_object *create_cq(struct ib_uverbs_file *file,
>         cq->uobject       = &obj->uobject;
>         cq->comp_handler  = ib_uverbs_comp_handler;
>         cq->event_handler = ib_uverbs_cq_event_handler;
> -       cq->cq_context    = &ev_file->ev_queue;
> +       cq->cq_context    = (cmd->comp_channel >= 0) ?
> +                               &ev_file->ev_queue : NULL;
>         atomic_set(&cq->usecnt, 0);
>
>         obj->uobject.object = cq;
> --
> 2.5.3
>

Nice catch, thanks.
I would prefer:
cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;

However, this fix is fine too.

Reviewed-by: Matan Barak <matanb@mellanox.com>

> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford Aug. 18, 2017, 6:06 p.m. UTC | #2
On Sun, 2017-08-06 at 11:08 +0300, Matan Barak wrote:
> On Tue, Aug 1, 2017 at 8:28 AM, Potnuri Bharat Teja <bharat@chelsio.c
> om> wrote:
> > Initializing cq_context with ev_queue in create_cq(), leads to NULL
> > pointer
> > dereference in ib_uverbs_comp_handler(), if application doesnot use
> > completion
> > channel. This patch fixes the cq_context initialization.
> > 
> > Fixes: 1e7710f3f65 ("IB/core: Change completion channel to use the
> > reworked")
> > Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> > ---
> >  drivers/infiniband/core/uverbs_cmd.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c
> > b/drivers/infiniband/core/uverbs_cmd.c
> > index 2c98533a0203..8c829522d8c4 100644
> > --- a/drivers/infiniband/core/uverbs_cmd.c
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -1015,7 +1015,8 @@ static struct ib_ucq_object *create_cq(struct
> > ib_uverbs_file *file,
> >         cq->uobject       = &obj->uobject;
> >         cq->comp_handler  = ib_uverbs_comp_handler;
> >         cq->event_handler = ib_uverbs_cq_event_handler;
> > -       cq->cq_context    = &ev_file->ev_queue;
> > +       cq->cq_context    = (cmd->comp_channel >= 0) ?
> > +                               &ev_file->ev_queue : NULL;
> >         atomic_set(&cq->usecnt, 0);
> > 
> >         obj->uobject.object = cq;
> > --
> > 2.5.3
> > 
> 
> Nice catch, thanks.
> I would prefer:
> cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;
> 
> However, this fix is fine too.
> 
> Reviewed-by: Matan Barak <matanb@mellanox.com>

I agree, I like your solution better.  Patch fixed up and applied.
Steve Wise Aug. 18, 2017, 8:51 p.m. UTC | #3
> 
> I agree, I like your solution better.  Patch fixed up and applied.
> 

Hey Doug, I see this in your -next branch, but it needs to hit 4.13-rc and stable, as it is a fatal regression that breaks user mode rdma applications.

Thanks,

Steve.


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Wise Aug. 21, 2017, 7:11 p.m. UTC | #4
ping...

> 
> >
> > I agree, I like your solution better.  Patch fixed up and applied.
> >
> 
> Hey Doug, I see this in your -next branch, but it needs to hit 4.13-rc and stable, as it
> is a fatal regression that breaks user mode rdma applications.
> 
> Thanks,
> 
> Steve.
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford Aug. 22, 2017, 12:44 a.m. UTC | #5
On 8/21/2017 3:11 PM, Steve Wise wrote:
> ping...
> 
>>
>>>
>>> I agree, I like your solution better.  Patch fixed up and applied.
>>>
>>
>> Hey Doug, I see this in your -next branch, but it needs to hit 4.13-rc and stable, as it
>> is a fatal regression that breaks user mode rdma applications.

OK, I'll get it into -rc then.
diff mbox

Patch

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 2c98533a0203..8c829522d8c4 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1015,7 +1015,8 @@  static struct ib_ucq_object *create_cq(struct ib_uverbs_file *file,
 	cq->uobject       = &obj->uobject;
 	cq->comp_handler  = ib_uverbs_comp_handler;
 	cq->event_handler = ib_uverbs_cq_event_handler;
-	cq->cq_context    = &ev_file->ev_queue;
+	cq->cq_context    = (cmd->comp_channel >= 0) ?
+				&ev_file->ev_queue : NULL;
 	atomic_set(&cq->usecnt, 0);
 
 	obj->uobject.object = cq;