summaryrefslogtreecommitdiff
path: root/mm/hmm.c
diff options
context:
space:
mode:
authorJason Gunthorpe <jgg@mellanox.com>2019-05-24 18:14:08 +0300
committerJason Gunthorpe <jgg@mellanox.com>2019-06-24 23:38:18 +0300
commit14331726a3c47bb1649dab155a84610f509d414e (patch)
tree9dc1a6341e79bdaf3c3ac93bd6fa23ac8c316eae /mm/hmm.c
parent2dcc3eb8ab50c9ca816cc60abfd94bea559d3e86 (diff)
downloadlinux-14331726a3c47bb1649dab155a84610f509d414e.tar.xz
mm/hmm: Remove confusing comment and logic from hmm_release
hmm_release() is called exactly once per hmm. ops->release() cannot accidentally trigger any action that would recurse back onto hmm->mirrors_sem. This fixes a use after-free race of the form: CPU0 CPU1 hmm_release() up_write(&hmm->mirrors_sem); hmm_mirror_unregister(mirror) down_write(&hmm->mirrors_sem); up_write(&hmm->mirrors_sem); kfree(mirror) mirror->ops->release(mirror) The only user we have today for ops->release is an empty function, so this is unambiguously safe. As a consequence of plugging this race drivers are not allowed to register/unregister mirrors from within a release op. Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Tested-by: Philip Yang <Philip.Yang@amd.com>
Diffstat (limited to 'mm/hmm.c')
-rw-r--r--mm/hmm.c28
1 files changed, 9 insertions, 19 deletions
diff --git a/mm/hmm.c b/mm/hmm.c
index c30aa9403dbe..b224ea635a77 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -130,26 +130,16 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
*/
WARN_ON(!list_empty_careful(&hmm->ranges));
- down_write(&hmm->mirrors_sem);
- mirror = list_first_entry_or_null(&hmm->mirrors, struct hmm_mirror,
- list);
- while (mirror) {
- list_del_init(&mirror->list);
- if (mirror->ops->release) {
- /*
- * Drop mirrors_sem so the release callback can wait
- * on any pending work that might itself trigger a
- * mmu_notifier callback and thus would deadlock with
- * us.
- */
- up_write(&hmm->mirrors_sem);
+ down_read(&hmm->mirrors_sem);
+ list_for_each_entry(mirror, &hmm->mirrors, list) {
+ /*
+ * Note: The driver is not allowed to trigger
+ * hmm_mirror_unregister() from this thread.
+ */
+ if (mirror->ops->release)
mirror->ops->release(mirror);
- down_write(&hmm->mirrors_sem);
- }
- mirror = list_first_entry_or_null(&hmm->mirrors,
- struct hmm_mirror, list);
}
- up_write(&hmm->mirrors_sem);
+ up_read(&hmm->mirrors_sem);
hmm_put(hmm);
}
@@ -279,7 +269,7 @@ void hmm_mirror_unregister(struct hmm_mirror *mirror)
struct hmm *hmm = mirror->hmm;
down_write(&hmm->mirrors_sem);
- list_del_init(&mirror->list);
+ list_del(&mirror->list);
up_write(&hmm->mirrors_sem);
hmm_put(hmm);
}