[PATCH] lib/test_hmm: fix memory leak in dmirror_migrate_to_system()

Hao Ge posted 1 patch 1 week, 4 days ago
lib/test_hmm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] lib/test_hmm: fix memory leak in dmirror_migrate_to_system()
Posted by Hao Ge 1 week, 4 days ago
Move the kvcalloc() calls after the early return checks to avoid
leaking src_pfns and dst_pfns when end < start or mmget_not_zero()
fails.

Fixes: 775465fd26a3 ("lib/test_hmm: add zone device private THP test infrastructure")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
Sashiko report: https://sashiko.dev/#/patchset/20260513082525.154036-1-hao.ge@linux.dev

It looks like the other issue reported by Sashiko (dmirror_devmem_fault()
leaking args.src/args.dst on migrate_vma_setup() failure) may have already
been addressed in mm-new by 84808b0e9f75 ("lib/test_hmm: fix error path in
dmirror_devmem_fault()").
---
 lib/test_hmm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 213504915737..0c02871b97ba 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -1111,9 +1111,6 @@ static int dmirror_migrate_to_system(struct dmirror *dmirror,
 	unsigned long *src_pfns;
 	unsigned long *dst_pfns;
 
-	src_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*src_pfns), GFP_KERNEL | __GFP_NOFAIL);
-	dst_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*dst_pfns), GFP_KERNEL | __GFP_NOFAIL);
-
 	start = cmd->addr;
 	end = start + size;
 	if (end < start)
@@ -1123,6 +1120,9 @@ static int dmirror_migrate_to_system(struct dmirror *dmirror,
 	if (!mmget_not_zero(mm))
 		return -EINVAL;
 
+	src_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*src_pfns), GFP_KERNEL | __GFP_NOFAIL);
+	dst_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*dst_pfns), GFP_KERNEL | __GFP_NOFAIL);
+
 	cmd->cpages = 0;
 	mmap_read_lock(mm);
 	for (addr = start; addr < end; addr = next) {
-- 
2.25.1
Re: [PATCH] lib/test_hmm: fix memory leak in dmirror_migrate_to_system()
Posted by Alistair Popple 1 week, 4 days ago
On 2026-05-28 at 11:13 +1000, Hao Ge <hao.ge@linux.dev> wrote...
> Move the kvcalloc() calls after the early return checks to avoid
> leaking src_pfns and dst_pfns when end < start or mmget_not_zero()
> fails.

Thanks.

Reviewed-by: Alistair Popple <apopple@nvidia.com>

> Fixes: 775465fd26a3 ("lib/test_hmm: add zone device private THP test infrastructure")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
> Sashiko report: https://sashiko.dev/#/patchset/20260513082525.154036-1-hao.ge@linux.dev
> 
> It looks like the other issue reported by Sashiko (dmirror_devmem_fault()
> leaking args.src/args.dst on migrate_vma_setup() failure) may have already
> been addressed in mm-new by 84808b0e9f75 ("lib/test_hmm: fix error path in
> dmirror_devmem_fault()").
> ---
>  lib/test_hmm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> index 213504915737..0c02871b97ba 100644
> --- a/lib/test_hmm.c
> +++ b/lib/test_hmm.c
> @@ -1111,9 +1111,6 @@ static int dmirror_migrate_to_system(struct dmirror *dmirror,
>  	unsigned long *src_pfns;
>  	unsigned long *dst_pfns;
>  
> -	src_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*src_pfns), GFP_KERNEL | __GFP_NOFAIL);
> -	dst_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*dst_pfns), GFP_KERNEL | __GFP_NOFAIL);
> -
>  	start = cmd->addr;
>  	end = start + size;
>  	if (end < start)
> @@ -1123,6 +1120,9 @@ static int dmirror_migrate_to_system(struct dmirror *dmirror,
>  	if (!mmget_not_zero(mm))
>  		return -EINVAL;
>  
> +	src_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*src_pfns), GFP_KERNEL | __GFP_NOFAIL);
> +	dst_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*dst_pfns), GFP_KERNEL | __GFP_NOFAIL);
> +
>  	cmd->cpages = 0;
>  	mmap_read_lock(mm);
>  	for (addr = start; addr < end; addr = next) {
> -- 
> 2.25.1
> 
>
Re: [PATCH] lib/test_hmm: fix memory leak in dmirror_migrate_to_system()
Posted by Balbir Singh 1 week, 3 days ago
On Thu, May 28, 2026 at 02:13:42PM +1000, Alistair Popple wrote:
> On 2026-05-28 at 11:13 +1000, Hao Ge <hao.ge@linux.dev> wrote...
> > Move the kvcalloc() calls after the early return checks to avoid
> > leaking src_pfns and dst_pfns when end < start or mmget_not_zero()
> > fails.
> 
> Thanks.
> 
> Reviewed-by: Alistair Popple <apopple@nvidia.com>

Thanks, might be late but

Reviewed-by: Balbir Singh <balbirs@nvidia.com>

> 
> > Fixes: 775465fd26a3 ("lib/test_hmm: add zone device private THP test infrastructure")
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Signed-off-by: Hao Ge <hao.ge@linux.dev>
> > ---
> > Sashiko report: https://sashiko.dev/#/patchset/20260513082525.154036-1-hao.ge@linux.dev
> > 
> > It looks like the other issue reported by Sashiko (dmirror_devmem_fault()
> > leaking args.src/args.dst on migrate_vma_setup() failure) may have already
> > been addressed in mm-new by 84808b0e9f75 ("lib/test_hmm: fix error path in
> > dmirror_devmem_fault()").
> > ---
> >  lib/test_hmm.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> > index 213504915737..0c02871b97ba 100644
> > --- a/lib/test_hmm.c
> > +++ b/lib/test_hmm.c
> > @@ -1111,9 +1111,6 @@ static int dmirror_migrate_to_system(struct dmirror *dmirror,
> >  	unsigned long *src_pfns;
> >  	unsigned long *dst_pfns;
> >  
> > -	src_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*src_pfns), GFP_KERNEL | __GFP_NOFAIL);
> > -	dst_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*dst_pfns), GFP_KERNEL | __GFP_NOFAIL);
> > -
> >  	start = cmd->addr;
> >  	end = start + size;
> >  	if (end < start)
> > @@ -1123,6 +1120,9 @@ static int dmirror_migrate_to_system(struct dmirror *dmirror,
> >  	if (!mmget_not_zero(mm))
> >  		return -EINVAL;
> >  
> > +	src_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*src_pfns), GFP_KERNEL | __GFP_NOFAIL);
> > +	dst_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*dst_pfns), GFP_KERNEL | __GFP_NOFAIL);
> > +
> >  	cmd->cpages = 0;
> >  	mmap_read_lock(mm);
> >  	for (addr = start; addr < end; addr = next) {
> > -- 
> > 2.25.1
> > 
> >