Re: [PATCH 3/4] mm/cma: reject CMA areas that overlap the kernel image

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/cma: __pa_symbol(_text) triggers BUG() on arm32 DEBUG_VIRTUAL
--

commit efca63a89d010b4bf63ea1571c8931374f2ec14d
Author: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>

mm/cma: reject CMA areas that overlap the kernel image

This commit updates cma_init_reserved_mem() to reject CMA regions that
overlap the kernel image range [_text, _end) so that cma_activate_area()
cannot free kernel image pages into MIGRATE_CMA.

> diff --git a/mm/cma.c b/mm/cma.c
> index 9c53b2b70dc8f..3231a53161cda 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -31,6 +31,8 @@
>  #include <linux/kmemleak.h>
>  #include <trace/events/cma.h>
>  
> +#include <asm/sections.h>
> +
>  #include "internal.h"
>  #include "cma.h"
>  #include "mm_init.h"
> @@ -68,6 +70,16 @@ bool cma_has_dynamic_base(const struct cma *cma)
>  	return test_bit(CMA_DYNAMIC_BASE, &cma->flags);
>  }
>  
> +static bool cma_overlaps_kernel_image(phys_addr_t base, phys_addr_t size)
> +{
> +	phys_addr_t end;
> +
> +	if (check_add_overflow(base, size, &end))
> +		return true;
> +
> +	return base < __pa_symbol(_end) && __pa_symbol(_text) < end;
> +}
> +
>  static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
>  					     unsigned int align_order)
>  {
> @@ -295,6 +307,15 @@ int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
>  	if (!size || !memblock_is_region_reserved(base, size))
>  		return -EINVAL;
>  
> +	if (cma_overlaps_kernel_image(base, size)) {
> +		phys_addr_t text = __pa_symbol(_text);
> +		phys_addr_t end = __pa_symbol(_end);
> +
> +		pr_err("area %s (%pa, size %pa) overlaps kernel image [%pa-%pa]\n",
> +		       name ?: "(unnamed)", &base, &size, &text, &end);
> +		return -EBUSY;
> +	}

[Severity: High]
Is __pa_symbol(_text) safe to call in architecture-independent CMA code?

In include/asm-generic/sections.h, _text is noted as architecture-specific:

    * _text, _data: architecture specific, don't use them in arch-independent code

On 32-bit ARM, arch/arm/kernel/vmlinux.lds.S places _text at the start of
.head.text before _stext:

    .head.text : {
        _text = .;
        HEAD_TEXT
    }

while arch/arm/include/asm/memory.h defines KERNEL_START starting at _stext
(or _sdata on CONFIG_XIP_KERNEL):

    #ifdef CONFIG_XIP_KERNEL
    #define KERNEL_START		_sdata
    #else
    #define KERNEL_START		_stext
    #endif
    #define KERNEL_END		_end

When CONFIG_DEBUG_VIRTUAL=y is enabled on 32-bit ARM, __pa_symbol(_text)
calls __phys_addr_symbol() in arch/arm/mm/physaddr.c:

    phys_addr_t __phys_addr_symbol(unsigned long x)
    {
        /* This is bounds checking against the kernel image only.
         * __pa_symbol should only be used on kernel symbol addresses.
         */
        VIRTUAL_BUG_ON(x < (unsigned long)KERNEL_START ||
                       x > (unsigned long)KERNEL_END);
        return __pa_symbol_nodebug(x);
    }

Because _text is below KERNEL_START on 32-bit ARM, won't calling
cma_init_reserved_mem() -> cma_overlaps_kernel_image() with
base < __pa_symbol(_end) trigger VIRTUAL_BUG_ON() during early boot?

In addition, on 32-bit ARM with CONFIG_XIP_KERNEL=y (where _text is linked
in ROM below PAGE_OFFSET while only [_sdata, _end) is in RAM) and on sparc64
(where [_text, _end) is linked at KERNBASE rather than in the PAGE_OFFSET
linear map), won't __pa_symbol(_text) also compute bogus physical addresses
for the kernel image?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103162429.2053989-1-radhey.shyam.pandey@amd.com?part=3