On Thu, Jun 06, 2019 at 04:54:41PM -0700, Ira Weiny wrote:
> On Thu, May 23, 2019 at 12:34:26PM -0300, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> >
> > mmu_notifier_unregister_no_release() is not a fence and the mmu_notifier
> > system will continue to reference hmm->mn until the srcu grace period
> > expires.
> >
> > Resulting in use after free races like this:
> >
> > CPU0 CPU1
> > __mmu_notifier_invalidate_range_start()
> > srcu_read_lock
> > hlist_for_each ()
> > // mn == hmm->mn
> > hmm_mirror_unregister()
> > hmm_put()
> > hmm_free()
> > mmu_notifier_unregister_no_release()
> > hlist_del_init_rcu(hmm-mn->list)
> > mn->ops->invalidate_range_start(mn, range);
> > mm_get_hmm()
> > mm->hmm = NULL;
> > kfree(hmm)
> > mutex_lock(&hmm->lock);
> >
> > Use SRCU to kfree the hmm memory so that the notifiers can rely on hmm
> > existing. Get the now-safe hmm struct through container_of and directly
> > check kref_get_unless_zero to lock it against free.
> >
> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> > include/linux/hmm.h | 1 +
> > mm/hmm.c | 25 +++++++++++++++++++------
> > 2 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > index 51ec27a8466816..8b91c90d3b88cb 100644
> > +++ b/include/linux/hmm.h
> > @@ -102,6 +102,7 @@ struct hmm {
> > struct mmu_notifier mmu_notifier;
> > struct rw_semaphore mirrors_sem;
> > wait_queue_head_t wq;
> > + struct rcu_head rcu;
> > long notifiers;
> > bool dead;
> > };
> > diff --git a/mm/hmm.c b/mm/hmm.c
> > index 816c2356f2449f..824e7e160d8167 100644
> > +++ b/mm/hmm.c
> > @@ -113,6 +113,11 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> > return NULL;
> > }
> >
> > +static void hmm_fee_rcu(struct rcu_head *rcu)
>
> NIT: "free"
>
> Other than that looks good.
>
> Reviewed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
Fixed in v2, thanks
Jason
