diff options
author | Jason Gunthorpe <jgg@mellanox.com> | 2019-05-24 12:14:08 -0300 |
---|---|---|
committer | Jason Gunthorpe <jgg@mellanox.com> | 2019-06-24 17:38:18 -0300 |
commit | 14331726a3c47bb1649dab155a84610f509d414e (patch) | |
tree | 9dc1a6341e79bdaf3c3ac93bd6fa23ac8c316eae /mm | |
parent | 2dcc3eb8ab50c9ca816cc60abfd94bea559d3e86 (diff) | |
download | linux-stable-14331726a3c47bb1649dab155a84610f509d414e.tar.gz linux-stable-14331726a3c47bb1649dab155a84610f509d414e.tar.bz2 linux-stable-14331726a3c47bb1649dab155a84610f509d414e.zip |
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')
-rw-r--r-- | mm/hmm.c | 28 |
1 files changed, 9 insertions, 19 deletions
@@ -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); } |