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>
This commit is contained in:
Jason Gunthorpe 2019-05-24 12:14:08 -03:00
parent 2dcc3eb8ab
commit 14331726a3
1 changed files with 9 additions and 19 deletions

View File

@ -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) {
down_read(&hmm->mirrors_sem);
list_for_each_entry(mirror, &hmm->mirrors, list) {
/*
* 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.
* Note: The driver is not allowed to trigger
* hmm_mirror_unregister() from this thread.
*/
up_write(&hmm->mirrors_sem);
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);
}