Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-09 Thread Jason Gunthorpe
On Thu, Jun 06, 2019 at 07:29:08PM -0700, John Hubbard wrote: > On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > > From: Jason Gunthorpe > ... > > diff --git a/mm/hmm.c b/mm/hmm.c > > index 8e7403f081f44a..547002f56a163d 100644 > > +++ b/mm/hmm.c > ... > > @@ -125,7 +130,7 @@ static void

Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-09 Thread John Hubbard
On 6/7/19 5:34 AM, Jason Gunthorpe wrote: > On Thu, Jun 06, 2019 at 07:29:08PM -0700, John Hubbard wrote: >> On 6/6/19 11:44 AM, Jason Gunthorpe wrote: >>> From: Jason Gunthorpe >> ... >>> diff --git a/mm/hmm.c b/mm/hmm.c >>> index 8e7403f081f44a..547002f56a163d 100644 >>> +++ b/mm/hmm.c >> ...

Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-09 Thread Jason Gunthorpe
On Fri, Jun 07, 2019 at 09:34:32AM -0300, Jason Gunthorpe wrote: > CH also pointed out a more elegant solution, which is to get the write > side of the mmap_sem during hmm_mirror_unregister - no notifier > callback can be running in this case. Then we delete the kref, srcu > and so forth. Oops,

Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-09 Thread John Hubbard
On 6/7/19 5:34 AM, Jason Gunthorpe wrote: > On Thu, Jun 06, 2019 at 07:29:08PM -0700, John Hubbard wrote: >> On 6/6/19 11:44 AM, Jason Gunthorpe wrote: >>> From: Jason Gunthorpe >> ... >>> @@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm) >>> >>> static void hmm_release(struct

Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-09 Thread Jason Gunthorpe
On Sat, Jun 08, 2019 at 01:49:48AM -0700, Christoph Hellwig wrote: > I still think sruct hmm should die. We already have a structure used > for additional information for drivers having crazly tight integration > into the VM, and it is called struct mmu_notifier_mm. We really need > to reuse

Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-09 Thread Ralph Campbell
On 6/6/19 11:44 AM, Jason Gunthorpe wrote: From: Jason Gunthorpe 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

[PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-07 Thread Jason Gunthorpe
From: Jason Gunthorpe 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

Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers

2019-06-07 Thread John Hubbard
On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > From: Jason Gunthorpe ... > diff --git a/mm/hmm.c b/mm/hmm.c > index 8e7403f081f44a..547002f56a163d 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c ... > @@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref) > mm->hmm = NULL; >