Commit 3df593bf authored by Jason Gunthorpe's avatar Jason Gunthorpe

IB/uverbs: Get rid of null_obj_type

If the method fails after calling rdma_explicit_destroy (eg if
copy_to_user faults) then it will trigger a kernel oops:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
PGD 800000000548d067 P4D 800000000548d067 PUD 54a0067 PMD 0
SMP PTI
CPU: 0 PID: 359 Comm: ibv_rc_pingpong Not tainted 4.18.0-rc1+ #28
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014
RIP: 0010:          (null)
Code: Bad RIP value.
RSP: 0018:ffffc900001a3bf0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff88000603bd00 RCX: 0000000000000003
RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffff88000603bd00
RBP: 0000000000000001 R08: ffffc900001a3cf8 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffc900001a3cf0
R13: 0000000000000000 R14: ffffc900001a3cf0 R15: 0000000000000000
FS:  00007fb00dda8700(0000) GS:ffff880007c00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 000000000548e004 CR4: 00000000003606b0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 ? rdma_lookup_put_uobject+0x22/0x50 [ib_uverbs]
 ? uverbs_finalize_object+0x3b/0x60 [ib_uverbs]
 ? uverbs_finalize_attrs+0x128/0x140 [ib_uverbs]
 ? ib_uverbs_cmd_verbs+0x698/0x7c0 [ib_uverbs]
 ? find_held_lock+0x2d/0x90
 ? __might_fault+0x39/0x90
 ? ib_uverbs_ioctl+0x111/0x1f0 [ib_uverbs]
 ? do_vfs_ioctl+0xa0/0x6d0
 ? trace_hardirqs_on_caller+0xed/0x180
 ? _raw_spin_unlock_irq+0x24/0x40
 ? syscall_trace_enter+0x138/0x1d0
 ? ksys_ioctl+0x35/0x60
 ? __x64_sys_ioctl+0x11/0x20
 ? do_syscall_64+0x5b/0x1c0
 ? entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is because the type was replaced with the null_type during explicit
destroy that cannot complete the destruction.

One of the side effects of replacing the type is to make the object
handle totally unreachable - so no other command could attempt to use
it, even though it remains on the uboject list.

We can get the same end result by just fully destroying the object inside
rdma_explicit_destroy and leaving the caller the residual kref for the
uobj with no attached HW object, and no presence in the ubojects list.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
Reviewed-by: default avatarLeon Romanovsky <leonro@mellanox.com>
parent ed288d74
...@@ -439,12 +439,17 @@ static int __must_check _rdma_remove_commit_uobject(struct ib_uobject *uobj, ...@@ -439,12 +439,17 @@ static int __must_check _rdma_remove_commit_uobject(struct ib_uobject *uobj,
struct ib_uverbs_file *ufile = uobj->ufile; struct ib_uverbs_file *ufile = uobj->ufile;
int ret; int ret;
if (!uobj->object)
return 0;
ret = uobj->type->type_class->remove_commit(uobj, why); ret = uobj->type->type_class->remove_commit(uobj, why);
if (ib_is_destroy_retryable(ret, why, uobj)) { if (ib_is_destroy_retryable(ret, why, uobj)) {
/* We couldn't remove the object, so just unlock the uobject */ /* We couldn't remove the object, so just unlock the uobject */
atomic_set(&uobj->usecnt, 0); atomic_set(&uobj->usecnt, 0);
uobj->type->type_class->lookup_put(uobj, true); uobj->type->type_class->lookup_put(uobj, true);
} else { } else {
uobj->object = NULL;
mutex_lock(&ufile->uobjects_lock); mutex_lock(&ufile->uobjects_lock);
list_del(&uobj->list); list_del(&uobj->list);
mutex_unlock(&ufile->uobjects_lock); mutex_unlock(&ufile->uobjects_lock);
...@@ -459,35 +464,13 @@ static int __must_check _rdma_remove_commit_uobject(struct ib_uobject *uobj, ...@@ -459,35 +464,13 @@ static int __must_check _rdma_remove_commit_uobject(struct ib_uobject *uobj,
int __must_check rdma_remove_commit_uobject(struct ib_uobject *uobj) int __must_check rdma_remove_commit_uobject(struct ib_uobject *uobj)
{ {
int ret; int ret;
struct ib_uverbs_file *ufile = uobj->ufile;
/* put the ref count we took at lookup_get */
uverbs_uobject_put(uobj);
/* Cleanup is running. Calling this should have been impossible */
if (!down_read_trylock(&ufile->cleanup_rwsem)) {
WARN(true, "ib_uverbs: Cleanup is running while removing an uobject\n");
return 0;
}
assert_uverbs_usecnt(uobj, true);
ret = _rdma_remove_commit_uobject(uobj, RDMA_REMOVE_DESTROY);
up_read(&ufile->cleanup_rwsem); ret = rdma_explicit_destroy(uobj);
/* Pairs with the lookup_get done by the caller */
rdma_lookup_put_uobject(uobj, true);
return ret; return ret;
} }
static int null_obj_type_class_remove_commit(struct ib_uobject *uobj,
enum rdma_remove_reason why)
{
return 0;
}
static const struct uverbs_obj_type null_obj_type = {
.type_class = &((const struct uverbs_obj_type_class){
.remove_commit = null_obj_type_class_remove_commit,
/* be cautious */
.needs_kfree_rcu = true}),
};
int rdma_explicit_destroy(struct ib_uobject *uobject) int rdma_explicit_destroy(struct ib_uobject *uobject)
{ {
int ret; int ret;
...@@ -499,14 +482,8 @@ int rdma_explicit_destroy(struct ib_uobject *uobject) ...@@ -499,14 +482,8 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
return 0; return 0;
} }
assert_uverbs_usecnt(uobject, true); assert_uverbs_usecnt(uobject, true);
ret = uobject->type->type_class->remove_commit(uobject, ret = _rdma_remove_commit_uobject(uobject, RDMA_REMOVE_DESTROY);
RDMA_REMOVE_DESTROY);
if (ret)
goto out;
uobject->type = &null_obj_type;
out:
up_read(&ufile->cleanup_rwsem); up_read(&ufile->cleanup_rwsem);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment