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

Bernd Schubert
2012-01-20T16:14:46Z

Hmm, I think we do have serious problem with the hole approach. While
the patch works for the kernel side, there is a problem with user space
libraries. So I monitored our daemons and noticed ibv_destroy_cq()
failed. The reason again seems to be the same issue as already fixed for
kernel qp's. So in __ibv_create_qp() (libibverbs/src/verbs.c):

> __ibv_create_qp()

> struct ibv_qp *qp = pd->context->ops.create_qp(pd, qp_init_attr);
>
> if (qp) {
> qp->context = pd->context;
> qp->qp_context = qp_init_attr->qp_context;
> qp->pd = pd;
> qp->send_cq = qp_init_attr->send_cq;
[...]

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)

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?

Thanks,
Bernd