[v4,hmm,00/12]

Message

Jason Gunthorpe June 24, 2019, 9 p.m. UTC

From: Jason Gunthorpe <[e-mail]>

This patch series arised out of discussions with Jerome when looking at the
ODP changes, particularly informed by use after free races we have already
found and fixed in the ODP code (thanks to syzkaller) working with mmu
notifiers, and the discussion with Ralph on how to resolve the lifetime model.

Overall this brings in a simplified locking scheme and easy to explain
lifetime model:

 If a hmm_range is valid, then the hmm is valid, if a hmm is valid then the mm
 is allocated memory.

 If the mm needs to still be alive (ie to lock the mmap_sem, find a vma, etc)
 then the mmget must be obtained via mmget_not_zero().

The use of unlocked reads on 'hmm->dead' are also eliminated in favour of
using standard mmget() locking to prevent the mm from being released. Many of
the debugging checks of !range->hmm and !hmm->mm are dropped in favour of
poison - which is much clearer as to the lifetime intent.

The trailing patches are just some random cleanups I noticed when reviewing
this code.

I'll apply this in the next few days - the only patch that doesn't have enough
Reviewed-bys is 'mm/hmm: Remove confusing comment and logic from hmm_release',
which had alot of questions, I still think it is good. If people really don't
like it I'll drop it.

Thanks to everyone who took time to look at this!

Jason Gunthorpe (12):
  mm/hmm: fix use after free with struct hmm in the mmu notifiers
  mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
  mm/hmm: Hold a mmgrab from hmm to mm
  mm/hmm: Simplify hmm_get_or_create and make it reliable
  mm/hmm: Remove duplicate condition test before wait_event_timeout
  mm/hmm: Do not use list*_rcu() for hmm->ranges
  mm/hmm: Hold on to the mmget for the lifetime of the range
  mm/hmm: Use lockdep instead of comments
  mm/hmm: Remove racy protection against double-unregistration
  mm/hmm: Poison hmm_range during unregister
  mm/hmm: Remove confusing comment and logic from hmm_release
  mm/hmm: Fix error flows in hmm_invalidate_range_start

 drivers/gpu/drm/nouveau/nouveau_svm.c |   2 +-
 include/linux/hmm.h                   |  52 +----
 kernel/fork.c                         |   1 -
 mm/hmm.c                              | 275 ++++++++++++--------------
 4 files changed, 130 insertions(+), 200 deletions(-)

Comments

Jason Gunthorpe June 29, 2019, 1:26 a.m. UTC | #1

On Mon, Jun 24, 2019 at 06:00:58PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <[e-mail]>
> 
> This patch series arised out of discussions with Jerome when looking at the
> ODP changes, particularly informed by use after free races we have already
> found and fixed in the ODP code (thanks to syzkaller) working with mmu
> notifiers, and the discussion with Ralph on how to resolve the lifetime model.
> 
> Overall this brings in a simplified locking scheme and easy to explain
> lifetime model:
> 
>  If a hmm_range is valid, then the hmm is valid, if a hmm is valid then the mm
>  is allocated memory.
> 
>  If the mm needs to still be alive (ie to lock the mmap_sem, find a vma, etc)
>  then the mmget must be obtained via mmget_not_zero().
> 
> The use of unlocked reads on 'hmm->dead' are also eliminated in favour of
> using standard mmget() locking to prevent the mm from being released. Many of
> the debugging checks of !range->hmm and !hmm->mm are dropped in favour of
> poison - which is much clearer as to the lifetime intent.
> 
> The trailing patches are just some random cleanups I noticed when reviewing
> this code.
> 
> I'll apply this in the next few days - the only patch that doesn't have enough
> Reviewed-bys is 'mm/hmm: Remove confusing comment and logic from hmm_release',
> which had alot of questions, I still think it is good. If people really don't
> like it I'll drop it.
> 
> Thanks to everyone who took time to look at this!
> 
> Jason Gunthorpe (12):
>   mm/hmm: fix use after free with struct hmm in the mmu notifiers
>   mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
>   mm/hmm: Hold a mmgrab from hmm to mm
>   mm/hmm: Simplify hmm_get_or_create and make it reliable
>   mm/hmm: Remove duplicate condition test before wait_event_timeout
>   mm/hmm: Do not use list*_rcu() for hmm->ranges
>   mm/hmm: Hold on to the mmget for the lifetime of the range
>   mm/hmm: Use lockdep instead of comments
>   mm/hmm: Remove racy protection against double-unregistration
>   mm/hmm: Poison hmm_range during unregister
>   mm/hmm: Remove confusing comment and logic from hmm_release
>   mm/hmm: Fix error flows in hmm_invalidate_range_start

I think we are done now, so applied to hmm.git, thank you to everyone.

I expect some conflicts in linux-next with the AMD DRM driver, we need
to decide how to handle them.

Jason