Messages in this thread | | | Subject | Re: [PATCH 04/15] mm/hmm: unregister mmu_notifier when last HMM client quit v2 | From | John Hubbard <> | Date | Wed, 21 Mar 2018 16:22:49 -0700 |
| |
On 03/21/2018 11:16 AM, jglisse@redhat.com wrote: > From: Jérôme Glisse <jglisse@redhat.com> > > This code was lost in translation at one point. This properly call > mmu_notifier_unregister_no_release() once last user is gone. This > fix the zombie mm_struct as without this patch we do not drop the > refcount we have on it. > > Changed since v1: > - close race window between a last mirror unregistering and a new > mirror registering, which could have lead to use after free() > kind of bug > > Signed-off-by: Jérôme Glisse <jglisse@redhat.com> > Cc: Evgeny Baskakov <ebaskakov@nvidia.com> > Cc: Ralph Campbell <rcampbell@nvidia.com> > Cc: Mark Hairgrove <mhairgrove@nvidia.com> > Cc: John Hubbard <jhubbard@nvidia.com> > --- > mm/hmm.c | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index 6088fa6ed137..f75aa8df6e97 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -222,13 +222,24 @@ int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm) > if (!mm || !mirror || !mirror->ops) > return -EINVAL; > > +again: > mirror->hmm = hmm_register(mm); > if (!mirror->hmm) > return -ENOMEM; > > down_write(&mirror->hmm->mirrors_sem); > - list_add(&mirror->list, &mirror->hmm->mirrors); > - up_write(&mirror->hmm->mirrors_sem); > + if (mirror->hmm->mm == NULL) { > + /* > + * A racing hmm_mirror_unregister() is about to destroy the hmm > + * struct. Try again to allocate a new one. > + */ > + up_write(&mirror->hmm->mirrors_sem); > + mirror->hmm = NULL;
This is being set outside of locks, so now there is another race with another hmm_mirror_register...
I'll take a moment and draft up what I have in mind here, which is a more symmetrical locking scheme for these routines.
thanks, -- John Hubbard NVIDIA
| |