Re: [PATCH v3] mm/secretmem: properly account locked pages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] __secretmem_account_pages() validates the faulting process's RLIMIT_MEMLOCK but charges the file creator's locked_vm, allowing limit bypass and privilege escalation.
- [High] Secretmem folios evicted from the page cache via posix_fadvise permanently leak locked_vm charges during the file's lifetime, leading to unbounded double-accounting.
- [Medium] The removal of proactive RLIMIT_MEMLOCK checks during mmap() changes the error behavior from -EAGAIN to a fatal SIGBUS during page faults, breaking userspace API semantics.
--

commit 40e3e15f3a115ea49fb48575254bb19c8bf4d03e
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm/secretmem: properly account locked pages

secretmem accounts folios by treating memory as if it were mlock()'d and
thus limited by the RLIMIT_MEMLOCK limit.

> diff --git a/mm/secretmem.c b/mm/secretmem.c
> index d29865075b6ea..384f5cfc457f9 100644
> --- a/mm/secretmem.c
> +++ b/mm/secretmem.c
[ ... ]
> +static bool __secretmem_account_pages(struct user_struct *user,
> +		unsigned long nr_pages)
> +{
> +	unsigned long page_limit, cur_pages, new_pages;
> +
> +	if (!nr_pages)
> +		return true;
> +
> +	page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> +
> +	cur_pages = atomic_long_read(&user->locked_vm);
> +	do {
> +		new_pages = cur_pages + nr_pages;
> +		if (new_pages > page_limit)
> +			return false;
> +	} while (!atomic_long_try_cmpxchg(&user->locked_vm,
> +					  &cur_pages, new_pages));
> +	return true;
> +}

[Severity: High]
Does this code check the faulting task's limit but charge the file creator's
locked_vm?

If an unprivileged user creates a secretmem fd and passes it to a privileged
process which then faults the memory, it appears rlimit(RLIMIT_MEMLOCK)
resolves to current (the privileged task), while the pages are charged to
state->user->locked_vm (the unprivileged creator).

Can this bypass the creator's constraints and permanently inflate their
locked memory footprint until the file is closed?

[ ... ]
> @@ -72,8 +133,15 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)
>  			goto out;
>  		}
>  
> +		if (!secretmem_account_folio(state, folio)) {
> +			folio_put(folio);
> +			ret = VM_FAULT_SIGBUS;
> +			goto out;
> +		}
> +

[Severity: High]
If a user unmaps the page and calls posix_fadvise(POSIX_FADV_DONTNEED) on the
fd, the clean folio is evicted and mapping->a_ops->free_folio
(secretmem_free_folio) is invoked.

Since secretmem_free_folio() isn't updated in this patch to unaccount the
pages, does this permanently leak locked_vm charges?

Could a user loop this (access page, unmap, posix_fadvise) to exhaust their
RLIMIT_MEMLOCK limit without actually using physical memory, denying them the
ability to use locked memory until the file is closed?

[ ... ]
>  static int secretmem_mmap_prepare(struct vm_area_desc *desc)
>  {
> -	const unsigned long len = vma_desc_size(desc);
> -
>  	if (!vma_desc_test_any(desc, VMA_SHARED_BIT, VMA_MAYSHARE_BIT))
>  		return -EINVAL;
>  
> -	vma_desc_set_flags(desc, VMA_LOCKED_BIT, VMA_DONTDUMP_BIT);
> -	if (!mlock_future_ok(desc->mm, /*is_vma_locked=*/ true, len))
> -		return -EAGAIN;
> +	vma_desc_set_flags(desc, VMA_DONTDUMP_BIT);
>  	desc->vm_ops = &secretmem_vm_ops;
>  
>  	return 0;

[Severity: Medium]
Does removing the proactive mlock_future_ok() check during mmap() break
userspace API semantics for applications managing memory constraints?

Applications relying on mmap() failing gracefully with -EAGAIN when limits
are exhausted will now see mmap() succeed, but will unexpectedly crash with a
SIGBUS signal during normal memory access in secretmem_fault().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-secretmem-accounting-v3-1-94cb04399510@kernel.org?part=1