Re: [PATCH v3 14/26] mm/init-mm: Annotate static information into meminspect

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Registering `swapper_pg_dir` with a size of `sizeof(void *)` restricts the generated crash dump to only the first 8 bytes of the root page directory.
- [Medium] Linker boundaries (`_text`, `_sinittext`, etc.) are registered with an arbitrary 8-byte size, which incorrectly dumps tiny fragments of executable code rather than exporting the symbol addresses.
- [Low] Taking the address of `swapper_pg_dir` causes a fatal compilation error on architectures where it is defined as an rvalue macro.
--

--- Patch [14]: [PATCH v3 14/26] mm/init-mm: Annotate static information into meminspect ---
commit a168e74f97589c651c261ad981c839eec643f005
Author: Eugen Hristev <ehristev@kernel.org>

mm/init-mm: Annotate static information into meminspect

This commit annotates vital static information such as init_mm, swapper_pg_dir,
and various linker boundary symbols into the meminspect inspection table. The
information is stored in a dedicated section for inspection.

> diff --git a/mm/init-mm.c b/mm/init-mm.c
> index 3e792aad76261..cf53058410b17 100644
> --- a/mm/init-mm.c
> +++ b/mm/init-mm.c
[ ... ]
> @@ -19,6 +21,13 @@
>  
>  const struct vm_operations_struct vma_dummy_vm_ops;
>  
> +MEMINSPECT_AREA_ENTRY(_sinittext, sizeof(void *));
> +MEMINSPECT_AREA_ENTRY(_einittext, sizeof(void *));
> +MEMINSPECT_AREA_ENTRY(_end, sizeof(void *));
> +MEMINSPECT_AREA_ENTRY(_text, sizeof(void *));
> +MEMINSPECT_AREA_ENTRY(_stext, sizeof(void *));
> +MEMINSPECT_AREA_ENTRY(_etext, sizeof(void *));

[Severity: Medium]
Are linker boundaries like _text intended to be registered as 8-byte
memory areas?

By registering these linker boundary addresses (e.g., _text) as memory areas
of size sizeof(void *), the kernel will include arbitrary 8-byte chunks of
memory from these addresses as PT_LOAD segments in the crash dump. This wastes
space and creates nonsensical micro-segments.

[ ... ]
> @@ -50,6 +59,9 @@ struct mm_struct init_mm = {
>  	INIT_MM_CONTEXT(init_mm)
>  };
>  
> +MEMINSPECT_SIMPLE_ENTRY(init_mm);
> +MEMINSPECT_AREA_ENTRY(swapper_pg_dir, sizeof(void *));

[Severity: High]
Does registering swapper_pg_dir with a size of sizeof(void *) restrict the
generated crash dump to only the first 8 bytes of the root page directory?

By passing sizeof(void *) instead of the actual page directory size (e.g.,
PAGE_SIZE or PTRS_PER_PGD * sizeof(pgd_t)), the dump will only contain the
first entry of the root page table. This breaks virtual-to-physical address
translation for vmalloc or kernel modules in post-mortem analysis tools like
crash.

[Severity: Low]
Will this break the build on architectures where swapper_pg_dir is defined
as an rvalue macro?

The MEMINSPECT_AREA_ENTRY macro takes the address of the provided symbol using
&(sym). On several noMMU architectures, swapper_pg_dir is defined as an rvalue
macro (e.g., ((pgd_t *) 0) or NULL). Taking the address of an rvalue results
in a compiler error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-meminspect-v3-v3-0-7aa5a0a74d5c@oss.qualcomm.com?part=14