RDMA/core: Ensure that rdma_user_mmap_entry_remove() is a fence
PATCH 1/1 RDMA/core: Ensure that rdma_user_mmap_entry_remove() is a fence
The set of entry->driver_removed is missing locking, protect it with xa_lock() which is held by the only reader.
Otherwise readers may continue to see driver_removed = false after rdma_user_mmap_entry_remove() returns and may continue to try and establish new mmaps.
--- drivers/infiniband/core/ib_core_uverbs.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/infiniband/core/ib_core_uverbs.c +2 -0
@@ -232,7 +232,9 @@ void rdma_user_mmap_entry_remove(struct rdma_user_mmap_entry *entry)if (!entry)return ; +xa_lock (&entry->ucontext->mmap_xa); entry->driver_removed = true; +xa_unlock (&entry->ucontext->mmap_xa);kref_put (&entry->ref, rdma_user_mmap_entry_free); }EXPORT_SYMBOL (rdma_user_mmap_entry_remove);
Signature · 1 line
2.24.1
Quoted 6 lines · Jason Gunthorpe
The set of
entry->driver_removedis missing locking, protect it withxa_lock()which is held by the only reader.Otherwise readers may continue to see
driver_removed= false afterrdma_user_mmap_entry_remove()returns and may continue to try and establish new mmaps.
That's kind of an inherent race regardless, isn't it?
LGTM,
Quoted 9 lines · Gal Pressman
Quoted 6 lines · Jason Gunthorpe
The set of
entry->driver_removedis missing locking, protect it withxa_lock()which is held by the only reader.Otherwise readers may continue to see
driver_removed= false afterrdma_user_mmap_entry_remove()returns and may continue to try and establish new mmaps.That's kind of an inherent race regardless, isn't it?
How do you mean? Once rdma_user_mmap_entry_remove() returns no new mmaps can be established, existing mmaps remain. The driver can rely on this, by, for instance, calling free_page
Jason
Quoted 23 lines
From: [email protected] <linux-rdma- [email protected]> On Behalf Of Jason Gunthorpe
The set of
entry->driver_removedis missing locking, protect it withxa_lock()which is held by the only reader.Otherwise readers may continue to see
driver_removed= false afterrdma_user_mmap_entry_remove()returns and may continue to try and establish new mmaps.Fixes 3411f9f01b76 ("RDMA/core: Create mmap database and cookie helperfunctions")
Signed-off-by Jason Gunthorpe <[email protected]>--- drivers/infiniband/core/ib_core_uverbs.c | 2 ++ 1 file changed, 2 insertions(+)drivers/infiniband/core/ib_core_uverbs.c +0
-0
drivers/infiniband/core/ib_core_uverbs.c void rdma_user_mmap_entry_remove(struct
Quoted 10 lines
drivers/infiniband/core/ib_core_uverbs.c +2
-1@@ -232,7 +232,9 @@ void rdma_user_mmap_entry_remove(struct rdma_user_mmap_entry *entry)if (!entry)return ; +xa_lock (&entry->ucontext->mmap_xa); entry->driver_removed = true; +xa_unlock (&entry->ucontext->mmap_xa);kref_put (&entry->ref, rdma_user_mmap_entry_free); }EXPORT_SYMBOL (rdma_user_mmap_entry_remove); --
Thanks,
Quoted 1 line
2.24.1
Jason
AppliedQuoted 7 lines · Jason Gunthorpe
The set of
entry->driver_removedis missing locking, protect it withxa_lock()which is held by the only reader.Otherwise readers may continue to see
driver_removed= false afterrdma_user_mmap_entry_remove()returns and may continue to try and establish new mmaps.
Quoted 7 lines
Fixes 3411f9f01b76 ("RDMA/core: Create mmap database and cookie helper functions")Signed-off-by Jason Gunthorpe <[email protected]>Reviewed-by Gal Pressman <[email protected]>Acked-by Michal Kalderon <[email protected]>--- drivers/infiniband/core/ib_core_uverbs.c | 2 ++ 1 file changed, 2 insertions(+)
Applied to for-next
Jason