[PATCH] fs/proc/task_mmu: pin mm_users in do_pagemap_scan()

[PATCH] fs/proc/task_mmu: pin mm_users in do_pagemap_scan()

[Ngoc Quy]

The PAGEMAP_SCAN ioctl takes mmap_read_lock() on an mm obtained from
/proc/<pid>/pagemap without holding a reference to mm_users.

proc_mem_open() deliberately grabs only mm_count, so a consumer that
touches the address space has to call mmget_not_zero() first. Every
other consumer in this file does -- m_start(), do_procmap_query(),
show_smaps_rollup() and pagemap_read() -- but do_pagemap_scan() does
not.

Without that reference the target can reach exit_mmap() concurrently,
and exit_mmap() calls unmap_vmas() while holding mmap_lock only for
reading, which does not exclude the scanner. For ordinary VMAs this is
harmless, as free_pgtables() and remove_vma() run afterwards under
mmap_write_lock. A shared hugetlb VMA is different: __hugetlb_zap_end()
frees the VMA's hugetlb_vma_lock inside the read-locked phase.

walk_hugetlb_range() can therefore load vma->vm_private_data, have the
lock freed underneath it, and then down_read() a freed rw_semaphore.
The load happens before the freeing side clears the pointer, so this is
a use-after-free and not a NULL dereference. Any unprivileged user can
reach it with no huge pages configured, as hugetlb_reserve_pages()
allocates the lock before the VM_NORESERVE early return.

Pin mm_users across the scan, the way the other consumers do. mmput()
from the exiting task can then no longer reach exit_mmap() while a walk
is in progress.

Fixes: 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ngoc Quy <ngocquy0700@gmail.com>
---
 fs/proc/task_mmu.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 5c54aebe2118..8825b01f5e38 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -3231,15 +3231,18 @@ static long do_pagemap_scan(struct mm_struct *mm, unsigned long uarg)
 	size_t n_ranges_out = 0;
 	int ret;
 
+	if (!mm || !mmget_not_zero(mm))
+		return -ESRCH;
+
 	ret = pagemap_scan_get_args(&p.arg, uarg);
 	if (ret)
-		return ret;
+		goto out_mmput;
 
 	p.masks_of_interest = p.arg.category_mask | p.arg.category_anyof_mask |
 			      p.arg.return_mask;
 	ret = pagemap_scan_init_bounce_buffer(&p);
 	if (ret)
-		return ret;
+		goto out_mmput;
 
 	for (walk_start = p.arg.start; walk_start < p.arg.end;
 			walk_start = p.arg.walk_end) {
@@ -3294,6 +3297,8 @@ static long do_pagemap_scan(struct mm_struct *mm, unsigned long uarg)
 		ret = -EFAULT;
 
 	kfree(p.vec_buf);
+out_mmput:
+	mmput(mm);
 	return ret;
 }
 

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.53.0

Re: [PATCH] fs/proc/task_mmu: pin mm_users in do_pagemap_scan()

[Andrew Morton]

On Wed, 14 Oct 2026 16:55:40 +0700 Ngoc Quy <ngocquy0700@gmail.com> wrote:

> The PAGEMAP_SCAN ioctl takes mmap_read_lock() on an mm obtained from
> /proc/<pid>/pagemap without holding a reference to mm_users.
> 
> proc_mem_open() deliberately grabs only mm_count, so a consumer that
> touches the address space has to call mmget_not_zero() first. Every
> other consumer in this file does -- m_start(), do_procmap_query(),
> show_smaps_rollup() and pagemap_read() -- but do_pagemap_scan() does
> not.
> 
> Without that reference the target can reach exit_mmap() concurrently,
> and exit_mmap() calls unmap_vmas() while holding mmap_lock only for
> reading, which does not exclude the scanner. For ordinary VMAs this is
> harmless, as free_pgtables() and remove_vma() run afterwards under
> mmap_write_lock. A shared hugetlb VMA is different: __hugetlb_zap_end()
> frees the VMA's hugetlb_vma_lock inside the read-locked phase.
> 
> walk_hugetlb_range() can therefore load vma->vm_private_data, have the
> lock freed underneath it, and then down_read() a freed rw_semaphore.
> The load happens before the freeing side clears the pointer, so this is
> a use-after-free and not a NULL dereference. Any unprivileged user can
> reach it with no huge pages configured, as hugetlb_reserve_pages()
> allocates the lock before the VM_NORESERVE early return.
> 
> Pin mm_users across the scan, the way the other consumers do. mmput()
> from the exiting task can then no longer reach exit_mmap() while a walk
> is in progress.

All looks good and checks out for me, thanks.

Except...

> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -3231,15 +3231,18 @@ static long do_pagemap_scan(struct mm_struct *mm, unsigned long uarg)
>  	size_t n_ranges_out = 0;
>  	int ret;
>  
> +	if (!mm || !mmget_not_zero(mm))
> +		return -ESRCH;

Why the new test for !mm?  The current call chain guarantees this is a
cant-happen and the current do_pagemap_scan will cheerfully oops if
mm==NULL.

If we start getting mm==NULL passed into the function then we want to
know why, and an oops is a good way of bringing attention, Quietly
accepting it reduces our ability to find stuff we did wrong,

Re: [PATCH] fs/proc/task_mmu: pin mm_users in do_pagemap_scan()

[David Hildenbrand (Arm)]

On 10/20/26 02:46, Andrew Morton wrote:
> On Wed, 14 Oct 2026 16:55:40 +0700 Ngoc Quy <ngocquy0700@gmail.com> wrote:
> 
>> The PAGEMAP_SCAN ioctl takes mmap_read_lock() on an mm obtained from
>> /proc/<pid>/pagemap without holding a reference to mm_users.
>>
>> proc_mem_open() deliberately grabs only mm_count, so a consumer that
>> touches the address space has to call mmget_not_zero() first. Every
>> other consumer in this file does -- m_start(), do_procmap_query(),
>> show_smaps_rollup() and pagemap_read() -- but do_pagemap_scan() does
>> not.
>>
>> Without that reference the target can reach exit_mmap() concurrently,
>> and exit_mmap() calls unmap_vmas() while holding mmap_lock only for
>> reading, which does not exclude the scanner. For ordinary VMAs this is
>> harmless, as free_pgtables() and remove_vma() run afterwards under
>> mmap_write_lock. A shared hugetlb VMA is different: __hugetlb_zap_end()
>> frees the VMA's hugetlb_vma_lock inside the read-locked phase.
>>
>> walk_hugetlb_range() can therefore load vma->vm_private_data, have the
>> lock freed underneath it, and then down_read() a freed rw_semaphore.
>> The load happens before the freeing side clears the pointer, so this is
>> a use-after-free and not a NULL dereference. Any unprivileged user can
>> reach it with no huge pages configured, as hugetlb_reserve_pages()
>> allocates the lock before the VM_NORESERVE early return.
>>
>> Pin mm_users across the scan, the way the other consumers do. mmput()
>> from the exiting task can then no longer reach exit_mmap() while a walk
>> is in progress.
> 
> All looks good and checks out for me, thanks.
> 
> Except...
> 
>> --- a/fs/proc/task_mmu.c
>> +++ b/fs/proc/task_mmu.c
>> @@ -3231,15 +3231,18 @@ static long do_pagemap_scan(struct mm_struct *mm, unsigned long uarg)
>>  	size_t n_ranges_out = 0;
>>  	int ret;
>>  
>> +	if (!mm || !mmget_not_zero(mm))
>> +		return -ESRCH;
> 
> Why the new test for !mm?  The current call chain guarantees this is a
> cant-happen and the current do_pagemap_scan will cheerfully oops if
> mm==NULL.

I'd just keep it for consistency with all the other checks in this file?

-- 
Cheers,

David

Re: [PATCH] fs/proc/task_mmu: pin mm_users in do_pagemap_scan()

[David Hildenbrand (Arm)]

On 10/14/26 11:55, Ngoc Quy wrote:
> The PAGEMAP_SCAN ioctl takes mmap_read_lock() on an mm obtained from
> /proc/<pid>/pagemap without holding a reference to mm_users.
> 
> proc_mem_open() deliberately grabs only mm_count, so a consumer that
> touches the address space has to call mmget_not_zero() first. Every
> other consumer in this file does -- m_start(), do_procmap_query(),
> show_smaps_rollup() and pagemap_read() -- but do_pagemap_scan() does
> not.
> 
> Without that reference the target can reach exit_mmap() concurrently,
> and exit_mmap() calls unmap_vmas() while holding mmap_lock only for
> reading, which does not exclude the scanner. For ordinary VMAs this is
> harmless, as free_pgtables() and remove_vma() run afterwards under
> mmap_write_lock. A shared hugetlb VMA is different: __hugetlb_zap_end()
> frees the VMA's hugetlb_vma_lock inside the read-locked phase.
> 
> walk_hugetlb_range() can therefore load vma->vm_private_data, have the
> lock freed underneath it, and then down_read() a freed rw_semaphore.
> The load happens before the freeing side clears the pointer, so this is
> a use-after-free and not a NULL dereference. Any unprivileged user can
> reach it with no huge pages configured, as hugetlb_reserve_pages()
> allocates the lock before the VM_NORESERVE early return.
> 
> Pin mm_users across the scan, the way the other consumers do. mmput()
> from the exiting task can then no longer reach exit_mmap() while a walk
> is in progress.
> 
> Fixes: 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Ngoc Quy <ngocquy0700@gmail.com>
> ---
>  fs/proc/task_mmu.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 5c54aebe2118..8825b01f5e38 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -3231,15 +3231,18 @@ static long do_pagemap_scan(struct mm_struct *mm, unsigned long uarg)
>  	size_t n_ranges_out = 0;
>  	int ret;
>  
> +	if (!mm || !mmget_not_zero(mm))
> +		return -ESRCH;
> +
>  	ret = pagemap_scan_get_args(&p.arg, uarg);
>  	if (ret)
> -		return ret;
> +		goto out_mmput;
>  
>  	p.masks_of_interest = p.arg.category_mask | p.arg.category_anyof_mask |
>  			      p.arg.return_mask;
>  	ret = pagemap_scan_init_bounce_buffer(&p);
>  	if (ret)
> -		return ret;
> +		goto out_mmput;
>  
>  	for (walk_start = p.arg.start; walk_start < p.arg.end;
>  			walk_start = p.arg.walk_end) {
> @@ -3294,6 +3297,8 @@ static long do_pagemap_scan(struct mm_struct *mm, unsigned long uarg)
>  		ret = -EFAULT;
>  
>  	kfree(p.vec_buf);
> +out_mmput:
> +	mmput(mm);
>  	return ret;
>  }

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David