Commit 0ac8903c authored by Jason Gunthorpe's avatar Jason Gunthorpe

RDMA/core: Allow the ioctl layer to abort a fully created uobject

While creating a uobject every create reaches a point where the uobject is
fully initialized. For ioctls that go on to copy_to_user this means they
need to open code the destruction of a fully created uobject - ie the
RDMA_REMOVE_DESTROY sort of flow.

Open coding this creates bugs, eg the CQ does not properly flush the
events list when it does its error unwind.

Provide a uverbs_finalize_uobj_create() function which indicates that the
uobject is fully initialized and that abort should call to destroy_hw to
destroy the uobj->object and related.

Methods can call this function if they go on to have error cases after
setting uobj->object. Once done those error cases can simply do return,
without an error unwind.

Link: https://lore.kernel.org/r/20200519072711.257271-2-leon@kernel.orgSigned-off-by: default avatarYishai Hadas <yishaih@mellanox.com>
Signed-off-by: default avatarLeon Romanovsky <leonro@mellanox.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
parent eafd47fc
...@@ -130,6 +130,17 @@ static int uverbs_destroy_uobject(struct ib_uobject *uobj, ...@@ -130,6 +130,17 @@ static int uverbs_destroy_uobject(struct ib_uobject *uobj,
lockdep_assert_held(&ufile->hw_destroy_rwsem); lockdep_assert_held(&ufile->hw_destroy_rwsem);
assert_uverbs_usecnt(uobj, UVERBS_LOOKUP_WRITE); assert_uverbs_usecnt(uobj, UVERBS_LOOKUP_WRITE);
if (reason == RDMA_REMOVE_ABORT_HWOBJ) {
reason = RDMA_REMOVE_ABORT;
ret = uobj->uapi_object->type_class->destroy_hw(uobj, reason,
attrs);
/*
* Drivers are not permitted to ignore RDMA_REMOVE_ABORT, see
* ib_is_destroy_retryable, cleanup_retryable == false here.
*/
WARN_ON(ret);
}
if (reason == RDMA_REMOVE_ABORT) { if (reason == RDMA_REMOVE_ABORT) {
WARN_ON(!list_empty(&uobj->list)); WARN_ON(!list_empty(&uobj->list));
WARN_ON(!uobj->context); WARN_ON(!uobj->context);
...@@ -647,11 +658,15 @@ void rdma_alloc_commit_uobject(struct ib_uobject *uobj, ...@@ -647,11 +658,15 @@ void rdma_alloc_commit_uobject(struct ib_uobject *uobj,
* object and anything else connected to uobj before calling this. * object and anything else connected to uobj before calling this.
*/ */
void rdma_alloc_abort_uobject(struct ib_uobject *uobj, void rdma_alloc_abort_uobject(struct ib_uobject *uobj,
struct uverbs_attr_bundle *attrs) struct uverbs_attr_bundle *attrs,
bool hw_obj_valid)
{ {
struct ib_uverbs_file *ufile = uobj->ufile; struct ib_uverbs_file *ufile = uobj->ufile;
uverbs_destroy_uobject(uobj, RDMA_REMOVE_ABORT, attrs); uverbs_destroy_uobject(uobj,
hw_obj_valid ? RDMA_REMOVE_ABORT_HWOBJ :
RDMA_REMOVE_ABORT,
attrs);
/* Matches the down_read in rdma_alloc_begin_uobject */ /* Matches the down_read in rdma_alloc_begin_uobject */
up_read(&ufile->hw_destroy_rwsem); up_read(&ufile->hw_destroy_rwsem);
...@@ -921,8 +936,8 @@ uverbs_get_uobject_from_file(u16 object_id, enum uverbs_obj_access access, ...@@ -921,8 +936,8 @@ uverbs_get_uobject_from_file(u16 object_id, enum uverbs_obj_access access,
} }
void uverbs_finalize_object(struct ib_uobject *uobj, void uverbs_finalize_object(struct ib_uobject *uobj,
enum uverbs_obj_access access, bool commit, enum uverbs_obj_access access, bool hw_obj_valid,
struct uverbs_attr_bundle *attrs) bool commit, struct uverbs_attr_bundle *attrs)
{ {
/* /*
* refcounts should be handled at the object level and not at the * refcounts should be handled at the object level and not at the
...@@ -945,7 +960,7 @@ void uverbs_finalize_object(struct ib_uobject *uobj, ...@@ -945,7 +960,7 @@ void uverbs_finalize_object(struct ib_uobject *uobj,
if (commit) if (commit)
rdma_alloc_commit_uobject(uobj, attrs); rdma_alloc_commit_uobject(uobj, attrs);
else else
rdma_alloc_abort_uobject(uobj, attrs); rdma_alloc_abort_uobject(uobj, attrs, hw_obj_valid);
break; break;
default: default:
WARN_ON(true); WARN_ON(true);
......
...@@ -64,8 +64,8 @@ uverbs_get_uobject_from_file(u16 object_id, enum uverbs_obj_access access, ...@@ -64,8 +64,8 @@ uverbs_get_uobject_from_file(u16 object_id, enum uverbs_obj_access access,
s64 id, struct uverbs_attr_bundle *attrs); s64 id, struct uverbs_attr_bundle *attrs);
void uverbs_finalize_object(struct ib_uobject *uobj, void uverbs_finalize_object(struct ib_uobject *uobj,
enum uverbs_obj_access access, bool commit, enum uverbs_obj_access access, bool hw_obj_valid,
struct uverbs_attr_bundle *attrs); bool commit, struct uverbs_attr_bundle *attrs);
int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx); int uverbs_output_written(const struct uverbs_attr_bundle *bundle, size_t idx);
......
...@@ -311,7 +311,7 @@ static int ib_uverbs_get_context(struct uverbs_attr_bundle *attrs) ...@@ -311,7 +311,7 @@ static int ib_uverbs_get_context(struct uverbs_attr_bundle *attrs)
return 0; return 0;
err_uobj: err_uobj:
rdma_alloc_abort_uobject(uobj, attrs); rdma_alloc_abort_uobject(uobj, attrs, false);
err_ucontext: err_ucontext:
kfree(attrs->context); kfree(attrs->context);
attrs->context = NULL; attrs->context = NULL;
......
...@@ -58,6 +58,7 @@ struct bundle_priv { ...@@ -58,6 +58,7 @@ struct bundle_priv {
DECLARE_BITMAP(uobj_finalize, UVERBS_API_ATTR_BKEY_LEN); DECLARE_BITMAP(uobj_finalize, UVERBS_API_ATTR_BKEY_LEN);
DECLARE_BITMAP(spec_finalize, UVERBS_API_ATTR_BKEY_LEN); DECLARE_BITMAP(spec_finalize, UVERBS_API_ATTR_BKEY_LEN);
DECLARE_BITMAP(uobj_hw_obj_valid, UVERBS_API_ATTR_BKEY_LEN);
/* /*
* Must be last. bundle ends in a flex array which overlaps * Must be last. bundle ends in a flex array which overlaps
...@@ -230,7 +231,8 @@ static void uverbs_free_idrs_array(const struct uverbs_api_attr *attr_uapi, ...@@ -230,7 +231,8 @@ static void uverbs_free_idrs_array(const struct uverbs_api_attr *attr_uapi,
for (i = 0; i != attr->len; i++) for (i = 0; i != attr->len; i++)
uverbs_finalize_object(attr->uobjects[i], uverbs_finalize_object(attr->uobjects[i],
spec->u2.objs_arr.access, commit, attrs); spec->u2.objs_arr.access, false, commit,
attrs);
} }
static int uverbs_process_attr(struct bundle_priv *pbundle, static int uverbs_process_attr(struct bundle_priv *pbundle,
...@@ -502,7 +504,9 @@ static void bundle_destroy(struct bundle_priv *pbundle, bool commit) ...@@ -502,7 +504,9 @@ static void bundle_destroy(struct bundle_priv *pbundle, bool commit)
uverbs_finalize_object( uverbs_finalize_object(
attr->obj_attr.uobject, attr->obj_attr.uobject,
attr->obj_attr.attr_elm->spec.u.obj.access, commit, attr->obj_attr.attr_elm->spec.u.obj.access,
test_bit(i, pbundle->uobj_hw_obj_valid),
commit,
&pbundle->bundle); &pbundle->bundle);
} }
...@@ -590,6 +594,8 @@ static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile, ...@@ -590,6 +594,8 @@ static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile,
sizeof(pbundle->bundle.attr_present)); sizeof(pbundle->bundle.attr_present));
memset(pbundle->uobj_finalize, 0, sizeof(pbundle->uobj_finalize)); memset(pbundle->uobj_finalize, 0, sizeof(pbundle->uobj_finalize));
memset(pbundle->spec_finalize, 0, sizeof(pbundle->spec_finalize)); memset(pbundle->spec_finalize, 0, sizeof(pbundle->spec_finalize));
memset(pbundle->uobj_hw_obj_valid, 0,
sizeof(pbundle->uobj_hw_obj_valid));
ret = ib_uverbs_run_method(pbundle, hdr->num_attrs); ret = ib_uverbs_run_method(pbundle, hdr->num_attrs);
bundle_destroy(pbundle, ret == 0); bundle_destroy(pbundle, ret == 0);
...@@ -784,3 +790,15 @@ int uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle, ...@@ -784,3 +790,15 @@ int uverbs_copy_to_struct_or_zero(const struct uverbs_attr_bundle *bundle,
} }
return uverbs_copy_to(bundle, idx, from, size); return uverbs_copy_to(bundle, idx, from, size);
} }
/* Once called an abort will call through to the type's destroy_hw() */
void uverbs_finalize_uobj_create(const struct uverbs_attr_bundle *bundle,
u16 idx)
{
struct bundle_priv *pbundle =
container_of(bundle, struct bundle_priv, bundle);
__set_bit(uapi_bkey_attr(uapi_key_attr(idx)),
pbundle->uobj_hw_obj_valid);
}
EXPORT_SYMBOL(uverbs_finalize_uobj_create);
...@@ -129,16 +129,12 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)( ...@@ -129,16 +129,12 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
obj->uevent.uobject.object = cq; obj->uevent.uobject.object = cq;
obj->uevent.uobject.user_handle = user_handle; obj->uevent.uobject.user_handle = user_handle;
rdma_restrack_uadd(&cq->res); rdma_restrack_uadd(&cq->res);
uverbs_finalize_uobj_create(attrs, UVERBS_ATTR_CREATE_CQ_HANDLE);
ret = uverbs_copy_to(attrs, UVERBS_ATTR_CREATE_CQ_RESP_CQE, &cq->cqe, ret = uverbs_copy_to(attrs, UVERBS_ATTR_CREATE_CQ_RESP_CQE, &cq->cqe,
sizeof(cq->cqe)); sizeof(cq->cqe));
if (ret) return ret;
goto err_cq;
return 0;
err_cq:
ib_destroy_cq_user(cq, uverbs_get_cleared_udata(attrs));
cq = NULL;
err_free: err_free:
kfree(cq); kfree(cq);
err_event_file: err_event_file:
......
...@@ -136,21 +136,15 @@ static int UVERBS_HANDLER(UVERBS_METHOD_DM_MR_REG)( ...@@ -136,21 +136,15 @@ static int UVERBS_HANDLER(UVERBS_METHOD_DM_MR_REG)(
uobj->object = mr; uobj->object = mr;
uverbs_finalize_uobj_create(attrs, UVERBS_ATTR_REG_DM_MR_HANDLE);
ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DM_MR_RESP_LKEY, &mr->lkey, ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DM_MR_RESP_LKEY, &mr->lkey,
sizeof(mr->lkey)); sizeof(mr->lkey));
if (ret) if (ret)
goto err_dereg; return ret;
ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DM_MR_RESP_RKEY, ret = uverbs_copy_to(attrs, UVERBS_ATTR_REG_DM_MR_RESP_RKEY,
&mr->rkey, sizeof(mr->rkey)); &mr->rkey, sizeof(mr->rkey));
if (ret)
goto err_dereg;
return 0;
err_dereg:
ib_dereg_mr_user(mr, uverbs_get_cleared_udata(attrs));
return ret; return ret;
} }
......
...@@ -2218,14 +2218,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_UMEM_REG)( ...@@ -2218,14 +2218,12 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_UMEM_REG)(
obj->mdev = dev->mdev; obj->mdev = dev->mdev;
uobj->object = obj; uobj->object = obj;
devx_obj_build_destroy_cmd(cmd.in, cmd.out, obj->dinbox, &obj->dinlen, &obj_id); devx_obj_build_destroy_cmd(cmd.in, cmd.out, obj->dinbox, &obj->dinlen, &obj_id);
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_DEVX_UMEM_REG_OUT_ID, &obj_id, sizeof(obj_id)); uverbs_finalize_uobj_create(attrs, MLX5_IB_ATTR_DEVX_UMEM_REG_HANDLE);
if (err)
goto err_umem_destroy;
return 0; err = uverbs_copy_to(attrs, MLX5_IB_ATTR_DEVX_UMEM_REG_OUT_ID, &obj_id,
sizeof(obj_id));
return err;
err_umem_destroy:
mlx5_cmd_exec(obj->mdev, obj->dinbox, obj->dinlen, cmd.out, sizeof(cmd.out));
err_umem_release: err_umem_release:
ib_umem_release(obj->umem); ib_umem_release(obj->umem);
err_obj_free: err_obj_free:
......
...@@ -6187,26 +6187,20 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_VAR_OBJ_ALLOC)( ...@@ -6187,26 +6187,20 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_VAR_OBJ_ALLOC)(
mmap_offset = mlx5_entry_to_mmap_offset(entry); mmap_offset = mlx5_entry_to_mmap_offset(entry);
length = entry->rdma_entry.npages * PAGE_SIZE; length = entry->rdma_entry.npages * PAGE_SIZE;
uobj->object = entry; uobj->object = entry;
uverbs_finalize_uobj_create(attrs, MLX5_IB_ATTR_VAR_OBJ_ALLOC_HANDLE);
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_VAR_OBJ_ALLOC_MMAP_OFFSET, err = uverbs_copy_to(attrs, MLX5_IB_ATTR_VAR_OBJ_ALLOC_MMAP_OFFSET,
&mmap_offset, sizeof(mmap_offset)); &mmap_offset, sizeof(mmap_offset));
if (err) if (err)
goto err; return err;
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_VAR_OBJ_ALLOC_PAGE_ID, err = uverbs_copy_to(attrs, MLX5_IB_ATTR_VAR_OBJ_ALLOC_PAGE_ID,
&entry->page_idx, sizeof(entry->page_idx)); &entry->page_idx, sizeof(entry->page_idx));
if (err) if (err)
goto err; return err;
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_VAR_OBJ_ALLOC_MMAP_LENGTH, err = uverbs_copy_to(attrs, MLX5_IB_ATTR_VAR_OBJ_ALLOC_MMAP_LENGTH,
&length, sizeof(length)); &length, sizeof(length));
if (err)
goto err;
return 0;
err:
rdma_user_mmap_entry_remove(&entry->rdma_entry);
return err; return err;
} }
...@@ -6320,26 +6314,20 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_UAR_OBJ_ALLOC)( ...@@ -6320,26 +6314,20 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_UAR_OBJ_ALLOC)(
mmap_offset = mlx5_entry_to_mmap_offset(entry); mmap_offset = mlx5_entry_to_mmap_offset(entry);
length = entry->rdma_entry.npages * PAGE_SIZE; length = entry->rdma_entry.npages * PAGE_SIZE;
uobj->object = entry; uobj->object = entry;
uverbs_finalize_uobj_create(attrs, MLX5_IB_ATTR_UAR_OBJ_ALLOC_HANDLE);
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_UAR_OBJ_ALLOC_MMAP_OFFSET, err = uverbs_copy_to(attrs, MLX5_IB_ATTR_UAR_OBJ_ALLOC_MMAP_OFFSET,
&mmap_offset, sizeof(mmap_offset)); &mmap_offset, sizeof(mmap_offset));
if (err) if (err)
goto err; return err;
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_UAR_OBJ_ALLOC_PAGE_ID, err = uverbs_copy_to(attrs, MLX5_IB_ATTR_UAR_OBJ_ALLOC_PAGE_ID,
&entry->page_idx, sizeof(entry->page_idx)); &entry->page_idx, sizeof(entry->page_idx));
if (err) if (err)
goto err; return err;
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_UAR_OBJ_ALLOC_MMAP_LENGTH, err = uverbs_copy_to(attrs, MLX5_IB_ATTR_UAR_OBJ_ALLOC_MMAP_LENGTH,
&length, sizeof(length)); &length, sizeof(length));
if (err)
goto err;
return 0;
err:
rdma_user_mmap_entry_remove(&entry->rdma_entry);
return err; return err;
} }
......
...@@ -69,17 +69,14 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_PP_OBJ_ALLOC)( ...@@ -69,17 +69,14 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_PP_OBJ_ALLOC)(
if (err) if (err)
goto err; goto err;
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_PP_OBJ_ALLOC_INDEX,
&pp_entry->index, sizeof(pp_entry->index));
if (err)
goto clean;
pp_entry->mdev = dev->mdev; pp_entry->mdev = dev->mdev;
uobj->object = pp_entry; uobj->object = pp_entry;
return 0; uverbs_finalize_uobj_create(attrs, MLX5_IB_ATTR_PP_OBJ_ALLOC_HANDLE);
err = uverbs_copy_to(attrs, MLX5_IB_ATTR_PP_OBJ_ALLOC_INDEX,
&pp_entry->index, sizeof(pp_entry->index));
return err;
clean:
mlx5_rl_remove_rate_raw(dev->mdev, pp_entry->index);
err: err:
kfree(pp_entry); kfree(pp_entry);
return err; return err;
......
...@@ -1491,6 +1491,11 @@ enum rdma_remove_reason { ...@@ -1491,6 +1491,11 @@ enum rdma_remove_reason {
RDMA_REMOVE_DRIVER_REMOVE, RDMA_REMOVE_DRIVER_REMOVE,
/* uobj is being cleaned-up before being committed */ /* uobj is being cleaned-up before being committed */
RDMA_REMOVE_ABORT, RDMA_REMOVE_ABORT,
/*
* uobj has been fully created, with the uobj->object set, but is being
* cleaned up before being comitted
*/
RDMA_REMOVE_ABORT_HWOBJ,
}; };
struct ib_rdmacg_object { struct ib_rdmacg_object {
......
...@@ -737,6 +737,9 @@ uverbs_attr_get_len(const struct uverbs_attr_bundle *attrs_bundle, u16 idx) ...@@ -737,6 +737,9 @@ uverbs_attr_get_len(const struct uverbs_attr_bundle *attrs_bundle, u16 idx)
return attr->ptr_attr.len; return attr->ptr_attr.len;
} }
void uverbs_finalize_uobj_create(const struct uverbs_attr_bundle *attrs_bundle,
u16 idx);
/* /*
* uverbs_attr_ptr_get_array_size() - Get array size pointer by a ptr * uverbs_attr_ptr_get_array_size() - Get array size pointer by a ptr
* attribute. * attribute.
......
...@@ -107,7 +107,7 @@ static inline void uobj_put_write(struct ib_uobject *uobj) ...@@ -107,7 +107,7 @@ static inline void uobj_put_write(struct ib_uobject *uobj)
static inline void uobj_alloc_abort(struct ib_uobject *uobj, static inline void uobj_alloc_abort(struct ib_uobject *uobj,
struct uverbs_attr_bundle *attrs) struct uverbs_attr_bundle *attrs)
{ {
rdma_alloc_abort_uobject(uobj, attrs); rdma_alloc_abort_uobject(uobj, attrs, false);
} }
static inline struct ib_uobject * static inline struct ib_uobject *
......
...@@ -139,7 +139,8 @@ void rdma_lookup_put_uobject(struct ib_uobject *uobj, ...@@ -139,7 +139,8 @@ void rdma_lookup_put_uobject(struct ib_uobject *uobj,
struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_api_object *obj, struct ib_uobject *rdma_alloc_begin_uobject(const struct uverbs_api_object *obj,
struct uverbs_attr_bundle *attrs); struct uverbs_attr_bundle *attrs);
void rdma_alloc_abort_uobject(struct ib_uobject *uobj, void rdma_alloc_abort_uobject(struct ib_uobject *uobj,
struct uverbs_attr_bundle *attrs); struct uverbs_attr_bundle *attrs,
bool hw_obj_valid);
void rdma_alloc_commit_uobject(struct ib_uobject *uobj, void rdma_alloc_commit_uobject(struct ib_uobject *uobj,
struct uverbs_attr_bundle *attrs); struct uverbs_attr_bundle *attrs);
......
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