Re: [PATCH] core/verb.c: fix kernel panic: always initialize struct ib_qp *qp->usecnt

Bernd Schubert
2012-01-23T15:11:02Z

On 01/20/2012 07:40 PM, Roland Dreier wrote:
> On Fri, Jan 20, 2012 at 8:14 AM, Bernd Schubert
> <bernd.schubert-mPn0NPGs4xGatNDF+KUbs4QuADTiUCJX@public.gmane.org> wrote:
>> I *guess* the qp allocated by pd->context->ops.create_qp() does not have
>> qp->usecnt initialized (not does it know anything about it). So its random
>> value will fail the destruction later. A simple workaround that would work
>> for us, is to extend the patch I send to
>>
>> diff --git a/drivers/infiniband/core/verbs.c
>> b/drivers/infiniband/core/verbs.c
>> index 602b1bd..fba1675 100644
>> --- a/drivers/infiniband/core/verbs.c
>> +++ b/drivers/infiniband/core/verbs.c
>> @@ -874,7 +874,7 @@ int ib_destroy_qp(struct ib_qp *qp)
>> struct ib_srq *srq;
>> int ret;
>>
>> - if (atomic_read(&qp->usecnt))
>> + if (qp->qp_type == IB_QPT_XRC_TGT&& atomic_read(&qp->usecnt))
>> return -EBUSY;
>>
>> if (qp->real_qp != qp)
>
> It looks like this is sufficient and correct without the other patch?
>
>>
>>
>> However, what is is with user space setting type to IB_QPT_XRC_TGT? I guess
>> this could be solved by letting the kernel zero the memory returned by
>> ->ops.create_qp(pd, qp_init_attr).
>> Btw, I didn't figure out yet, how this translates at all in kernel space? Is
>> this op directly going to the device driver?
>>
>> But even if we are properly going to initialize the qp, what is with user
>> space mischievously trying to crash the system by manipulating struct ib_qp
>> *qp?
>
> I don't follow this. Isn't *qp completely allocated and manipulated
> in the kernel? How can userspace touch it except by having the
> kernel do something via the uverbs interface?

Sorry, I first didn't see the ib_uverbs_create_qp() interface and
copy_from/to_user() there.

Cheers,
Bernd