Re: [RFC PATCH 5/9] arm64: mm: Permit permissions changes on huge vmappings

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 [off-list ref]

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.

[ ... ]

[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