Linux Trace Kernel
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ard Biesheuvel" <ardb+git@google.com>
Cc: linux-trace-kernel@vger.kernel.org, linux-modules@vger.kernel.org
Subject: Re: [RFC PATCH 5/9] arm64: mm: Permit permissions changes on huge vmappings
Date: Sat, 22 Aug 2026 14:10:57 +0000	[thread overview]
Message-ID: <20260822141057.D24451F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260822135323.795946-16-ardb+git@google.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Live splitting of huge vmalloc mappings silently fails and triggers a kernel warning.
- [High] Permission changes on small, PTE-mapped vmalloc areas are incorrectly rejected.
- [High] Improper use of `WARN_ON_ONCE` for a hardware capability check causes unwarranted kernel log spam and potential panic.
- [Low] Implicit declaration of function `system_supports_bbml2_noabort()` breaks the build.
--

--- Patch [5]: [RFC PATCH 5/9] arm64: mm: Permit permissions changes on huge vmappings ---
commit e67f32e911a059a6f372cf2818088d9fde53ee7f
Author: Ard Biesheuvel <ardb@kernel.org>

arm64: mm: Permit permissions changes on huge vmappings

This commit allows permission changes on huge vmappings without splitting
when the region is PMD-aligned or when the system supports live splitting.

> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index bbe98ac9ad8c6..20ff9cb273c11 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c

[ ... ]

> @@ -179,7 +177,16 @@ static int change_memory_common(unsigned long addr, int numpages,
>  	if (!area ||
>  	    ((unsigned long)kasan_reset_tag((void *)end) >
>  	     (unsigned long)kasan_reset_tag(area->addr) + area->size) ||
> -	    ((area->flags & (VM_ALLOC | VM_ALLOW_HUGE_VMAP)) != VM_ALLOC))
> +	    !(area->flags & VM_ALLOC))
>                 return -EINVAL;
>  
> +	/*
> +	 * Disallow VM_ALLOW_HUGE_VMAP mappings unless the region is PMD
> +	 * aligned, or splitting live huge mappings is supported.
> +	 */
> +	if ((area->flags & VM_ALLOW_HUGE_VMAP) &&
> +	   ((start % PMD_SIZE) || (size % PMD_SIZE)) &&
> +	   WARN_ON_ONCE(!system_supports_bbml2_noabort()))

[Severity: Low]
Does the function system_supports_bbml2_noabort() exist in the codebase? It
appears to be missing a definition or declaration, which might cause the
kernel to fail to compile due to an implicit function declaration.

[Severity: High]
Is WARN_ON_ONCE() the right choice for a hardware capability check here?

If an unaligned permission change is requested on an older system lacking
BBML2 support, this will trigger a warning stack trace. If panic_on_warn
is enabled, an unprivileged user loading a BPF program could trigger a
kernel panic. Should this condition fail silently instead?

[Severity: High]
Does checking area->flags & VM_ALLOW_HUGE_VMAP incorrectly reject small,
PTE-mapped vmalloc areas?

Allocators like execmem use VM_ALLOW_HUGE_VMAP as an allocation hint. If
an allocation is smaller than PMD_SIZE, it falls back to PTE mappings but
retains the VM_ALLOW_HUGE_VMAP flag in area->flags.

When change_memory_common() processes this on older hardware without
BBML2, it sees the flag alongside an unaligned size, and rejects the
request with -EINVAL, even though the page table is already PTE-mapped
and requires no splitting.

Should we check the actual page table granularity, such as inspecting
area->page_order, instead of just the allocation flag?

[Severity: High]
If this condition passes because system_supports_bbml2_noabort() is true,
will the PMD actually be split later?

change_memory_common() delegates to __change_memory_common() which
calls update_range_prot() and eventually split_kernel_leaf_mapping().

Looking at split_kernel_leaf_mapping() in arch/arm64/mm/mmu.c:

if (!linear_map_requires_bbml3 || is_kfence_address((void *)start))
    return 0;

if (!system_supports_bbml3()) {
    ...
    return 0;
}

This evaluates system_supports_bbml3() and linear_map_requires_bbml3,
ignoring both the BBML2 capability and the vmalloc context. If those are
false (for example, if rodata is off), it returns 0 without splitting the
PMD.

The subsequent lockless page table walk would then encounter an unsplit PMD
spanning a larger range than requested, resulting in a warning and a
failure.

> +		return -EINVAL;
>  
>  	if (!numpages)
>  		return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822135323.795946-11-ardb+git@google.com?part=5

  reply	other threads:[~2026-08-22 14:10 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-22 13:53 [RFC PATCH 0/9] arm64: Allocate .text and .init.text together Ard Biesheuvel
2026-08-22 13:53 ` [RFC PATCH 1/9] mm: execmem: Add API to split an existing execmem cache allocation Ard Biesheuvel
2026-08-22 13:53 ` [RFC PATCH 2/9] mm: execmem: Allow huge vmappings to be avoided for execmem caches Ard Biesheuvel
2026-08-22 14:10   ` sashiko-bot
2026-08-22 13:53 ` [RFC PATCH 3/9] module: Place MOD_TEXT before MOD_INIT_TEXT in enumeration Ard Biesheuvel
2026-08-22 14:05   ` sashiko-bot
2026-08-22 13:53 ` [RFC PATCH 4/9] module: Allocate MOD_INIT_TEXT from the MOD_TEXT ROX allocation Ard Biesheuvel
2026-08-22 13:53 ` [RFC PATCH 5/9] arm64: mm: Permit permissions changes on huge vmappings Ard Biesheuvel
2026-08-22 14:10   ` sashiko-bot [this message]
2026-08-23 16:52   ` Adrian Barnaś
2026-08-22 13:53 ` [RFC PATCH 6/9] arm64: Enable the execmem ROX cache for module text Ard Biesheuvel
2026-08-22 14:13   ` sashiko-bot
2026-08-23 16:46   ` Adrian Barnaś
2026-08-22 13:53 ` [RFC PATCH 7/9] arm64: ftrace: Revert "fix unreachable PLT for ftrace_caller ..." Ard Biesheuvel
2026-08-22 13:53 ` [RFC PATCH 8/9] arm64: module: Combine init and core PLT entries again Ard Biesheuvel
2026-08-22 14:12   ` sashiko-bot
2026-08-22 13:53 ` [RFC PATCH 9/9] arm64: ftrace: Simplify PLT handling Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260822141057.D24451F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ardb+git@google.com \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox