RDMA/uverbs: Remove needs_kfree_rcu from uverbs_obj_type_class
PATCH 1/1 RDMA/uverbs: Remove needs_kfree_rcu from uverbs_obj_type_class
After device disassociation the uapi_objects are destroyed and freed, however it is still possible that core code can be holding a kref on the uobject. When it finally goes to uverbs_uobject_free() via the kref_put() it can trigger a use-after-free on the uapi_object.
Since needs_kfree_rcu is a micro optimization that only benefits file uobjects, just get rid of it. There is no harm in using kfree_rcu even if it isn't required, and the number of involved objects is small.
--- drivers/infiniband/core/rdma_core.c | 23 +---------------------- include/rdma/uverbs_types.h | 1 - 2 files changed, 1 insertion(+), 23 deletions(-)
This should go before the 'refactoring fd usage' series as more testing has shown the reworked code can trivially trigger this existing bug.
drivers/infiniband/core/rdma_core.c +1 -22
@@ -49,13 +49,7 @@ void uverbs_uobject_get(struct ib_uobject *uobject)static void uverbs_uobject_free (struct kref *ref) { -struct ib_uobject *uobj = - container_of(ref, struct ib_uobject, ref); - -if (uobj->uapi_object->type_class->needs_kfree_rcu) -kfree_rcu (uobj, rcu); -else -kfree (uobj); + kfree_rcu(container_of(ref, struct ib_uobject, ref), rcu); }void uverbs_uobject_put (struct ib_uobject *uobject)
@@ -744,20 +738,6 @@ const struct uverbs_obj_type_class uverbs_idr_class = { .lookup_put = lookup_put_idr_uobject, .destroy_hw = destroy_hw_idr_uobject, .remove_handle = remove_handle_idr_uobject, -/* -* When we destroy an object, we first just lock it for WRITE and -* actually DESTROY it in the finalize stage. So, the problematic -* scenario is when we just started the finalize stage of the -* destruction (nothing was executed yet). Now, the other thread -* fetched the object for READ access, but it didn't lock it yet. -* The DESTROY thread continues and starts destroying the object. -* When the other thread continue - without the RCU, it would -* access freed memory. However, the rcu_read_lock delays the free -* until the rcu_read_lock of the READ operation quits. Since the -* exclusive lock of the object is still taken by the DESTROY flow, the -* READ operation will get -EBUSY and it'll just bail out. -*/ - .needs_kfree_rcu = true, };EXPORT_SYMBOL (uverbs_idr_class);
@@ -920,7 +900,6 @@ const struct uverbs_obj_type_class uverbs_fd_class = { .lookup_put = lookup_put_fd_uobject, .destroy_hw = destroy_hw_fd_uobject, .remove_handle = remove_handle_fd_uobject, - .needs_kfree_rcu = false, };EXPORT_SYMBOL (uverbs_fd_class);
include/rdma/uverbs_types.h +0 -1
@@ -98,7 +98,6 @@ struct uverbs_obj_type_class {enum rdma_remove_reason why, struct uverbs_attr_bundle *attrs);void (*remove_handle)(struct ib_uobject *uobj); -u8 needs_kfree_rcu; };struct uverbs_obj_type {
Signature · 1 line
2.24.1
Jason Gunthorpe
AppliedQuoted 11 lines · Jason Gunthorpe
After device disassociation the
uapi_objectsare destroyed and freed, however it is still possible that core code can be holding a kref on the uobject. When it finally goes touverbs_uobject_free()via thekref_put()it can trigger a use-after-free on theuapi_object.Since
needs_kfree_rcuis a micro optimization that only benefits file uobjects, just get rid of it. There is no harm in usingkfree_rcueven if it isn't required, and the number of involved objects is small.Signed-off-by Michael Guralnik <[email protected]>Signed-off-by Jason Gunthorpe <[email protected]>
Quoted 8 lines
--- drivers/infiniband/core/rdma_core.c | 23 +---------------------- include/rdma/uverbs_types.h | 1 - 2 files changed, 1 insertion(+), 23 deletions(-)This should go before the 'refactoring fd usage' series as more testing has shown the reworked code can trivially trigger this existing bug.
Applied to for-next
Though it seems this might not be an existing problem as none of the existing kref users can outlive disassociation. Nevertheless it is very surprising that the kref becomes a segfault after disassociation.
Jason