Re: [PATCH 5/5] mm/gup: limit pagecache folio refcount when grabbing references

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] mm/gup: try_get_folio() calls capable() with IRQs disabled in
  GUP-fast
--

commit 9290efad216cdf3b5ee1cdfafab0a5d9928a1976
Author: David Hildenbrand (Arm) <david@kernel.org>

mm/gup: limit pagecache folio refcount when grabbing references

This commit replaces the negative refcount safety checks in try_get_folio()
and try_grab_folio() with folio_ref_count_over_limit() to enforce folio
refcount limits in fast and slow GUP before acquiring references or pins.

> diff --git a/mm/gup.c b/mm/gup.c
> index 2ab42fc6834e5..db9db6b2a4662 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -77,7 +77,7 @@ static inline struct folio *try_get_folio(struct page *page, int refs)
>  
>  retry:
>  	folio = page_folio(page);
> -	if (WARN_ON_ONCE(folio_ref_count(folio) < 0))
> +	if (unlikely(folio_ref_count_over_limit(folio)))
>  		return NULL;

[Severity: High]
Is it safe for try_get_folio() to call folio_ref_count_over_limit() in the
fast GUP path?

In mm/gup.c, gup_fast() disables hardware interrupts across the page table
walk:

mm/gup.c:gup_fast() {
	...
	local_irq_save(flags);
	nr_pinned = gup_fast_pgd_range(start, end, gup_flags, pages);
	local_irq_restore(flags);
	...
}

which calls into try_grab_folio_fast() -> try_get_folio() with IRQs
disabled:

mm/gup.c:try_grab_folio_fast() {
	...
	/* Raise warn if it is not called in fast GUP */
	VM_WARN_ON_ONCE(!irqs_disabled());
	...
}

Furthermore, get_user_page_fast_only() -> gup_fast() -> try_get_folio() is
invoked from hardirq and NMI contexts, such as perf_prepare_sample() ->
perf_virt_to_phys() in kernel/events/core.c.

When folio_ref_count(folio) is between FOLIO_REF_COUNT_LIMIT and
FOLIO_REF_COUNT_ADMIN_LIMIT and current is not a kernel thread,
folio_ref_count_over_limit() in include/linux/mm.h calls
capable(CAP_SYS_RESOURCE):

include/linux/mm.h:folio_ref_count_over_limit() {
	const unsigned int ref_count = folio_ref_count(folio);

	if (likely(ref_count < FOLIO_REF_COUNT_LIMIT))
		return false;
	if (ref_count >= FOLIO_REF_COUNT_ADMIN_LIMIT)
		return true;
	/* Background threads must not consume admin reserves. */
	return (current->flags & PF_KTHREAD) || !capable(CAP_SYS_RESOURCE);
}

Calling capable(CAP_SYS_RESOURCE) -> ns_capable_common() ->
security_capable() in these contexts appears to cause several issues:

1. LSM capability hooks (such as selinux_capable() and apparmor_capable())
   perform audit logging via common_lsm_audit(), which allocates memory and
   acquires spinlock_t, rwlock_t, and SLUB local_lock_t locks. On
   CONFIG_PREEMPT_RT, these are sleeping rt_mutex locks, so acquiring them
   with hardware interrupts disabled in gup_fast() will sleep in an invalid
   atomic context.

2. When get_user_page_fast_only() is called from hardirq or NMI context,
   acquiring non-IRQ/NMI-safe locks in the LSM, audit, or SLUB paths can
   self-deadlock if the interrupted context on the same CPU already holds
   one of those locks.

3. In security/apparmor/capability.c, audit_caps() accesses the per-CPU
   audit_cache using get_cpu_var(), which only disables preemption:

   security/apparmor/capability.c:audit_caps() {
	...
	ent = &get_cpu_var(audit_cache);
	if (ad->subj_cred == ent->ad_subj_cred &&
	    ktime_get_ns() <= ent->ktime_ns_expiration[cap]) {
		...
	} else {
		put_cred(ent->ad_subj_cred);
		ent->ad_subj_cred = get_cred(ad->subj_cred);
		ent->ktime_ns_expiration[cap] = ...;
	}
	put_cpu_var(audit_cache);
	...
   }

   If an IRQ or NMI invokes get_user_page_fast_only() while the interrupted
   context is inside audit_caps() on the same CPU, both contexts can race on
   ent->ad_subj_cred and cause a double put_cred().

4. In kernel/capability.c, ns_capable_common() updates current->flags
   non-atomically:

   kernel/capability.c:ns_capable_common() {
	...
	capable = security_capable(current_cred(), ns, cap, opts);
	if (capable == 0) {
		current->flags |= PF_SUPERPRIV;
		return true;
	}
	...
   }

   When called from hardirq or NMI context, this mutates the interrupted
   task's flags and can race with process-context updates to current->flags.

Should try_get_folio() avoid calling capable() in fast GUP (for example, by
falling back to slow GUP once the folio refcount reaches
FOLIO_REF_COUNT_LIMIT)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-refcount_overflow_stable-v1-0-70b9c655da3e@kernel.org?part=5