[PATCH] mm/hmm: Fix struct hmm memory leak

[PATCH] mm/hmm: Fix struct hmm memory leak

[rcampbell]

From: Ralph Campbell <rcampbell@nvidia.com>

The patch [1] introduced reference counting on struct hmm and works
fine when calling hmm_mirror_register() and hmm_mirror_unregister().
However, when a process exits without explicitly unregistering,
the MMU notifier callback hmm_release() doesn't release the mirror->hmm
reference and thus leaks the struct hmm allocation.
Fix this by releasing the reference in hmm_release().

[1] https://marc.info/?l=linux-mm&m=154878089214597&w=2
    ("mm/hmm: use reference counting for HMM struct")

Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: John Hubbard <jhubbard@nvidia.com>
---
 mm/hmm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/hmm.c b/mm/hmm.c
index 3c9781037918..50523df6ea0c 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -179,6 +179,8 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 			mirror->ops->release(mirror);
 			down_write(&hmm->mirrors_sem);
 		}
+		hmm_put(mirror->hmm);
+		mirror->hmm = NULL;
 		mirror = list_first_entry_or_null(&hmm->mirrors,
 						  struct hmm_mirror, list);
 	}
-- 
2.17.2

Re: [PATCH] mm/hmm: Fix struct hmm memory leak

[Jerome Glisse]

On Fri, Feb 15, 2019 at 01:59:22PM -0800, rcampbell@nvidia.com wrote:
> From: Ralph Campbell <rcampbell@nvidia.com>
> 
> The patch [1] introduced reference counting on struct hmm and works
> fine when calling hmm_mirror_register() and hmm_mirror_unregister().
> However, when a process exits without explicitly unregistering,
> the MMU notifier callback hmm_release() doesn't release the mirror->hmm
> reference and thus leaks the struct hmm allocation.
> Fix this by releasing the reference in hmm_release().
> 
> [1] https://marc.info/?l=linux-mm&m=154878089214597&w=2
>     ("mm/hmm: use reference counting for HMM struct")

NAK we do not want to free stuff from underneath the driver that
was the whole point of the refcounting. Instead for driver that
want to free their mirror from the release call back can call the
hmm_mirror_unregister() function safely. Sorry if that was not
clear.

> 
> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: "Jérôme Glisse" <jglisse@redhat.com>
> Cc: John Hubbard <jhubbard@nvidia.com>
> ---
>  mm/hmm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 3c9781037918..50523df6ea0c 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -179,6 +179,8 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>  			mirror->ops->release(mirror);
>  			down_write(&hmm->mirrors_sem);
>  		}
> +		hmm_put(mirror->hmm);
> +		mirror->hmm = NULL;
>  		mirror = list_first_entry_or_null(&hmm->mirrors,
>  						  struct hmm_mirror, list);
>  	}
> -- 
> 2.17.2
>