Re: [PATCH v3 12/26] kernel/vmcore_info: Register dynamic information into meminspect

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The meminspect registration captures a stale size and physical address for `vmcoreinfo_data` at initialization time, causing dynamically appended crash-time data to be truncated and safecopy buffer redirects to be missed.
- [High] Registering the unaligned vmcoreinfo_size causes the resulting meminspect ELF core image to be misaligned and corrupted upon concatenation.
--

--- Patch [12]: [PATCH v3 12/26] kernel/vmcore_info: Register dynamic information into meminspect ---
commit 8bf40adf00adb32d4b5c8192b6dcc0d380039441
Author: Eugen Hristev <ehristev@kernel.org>

kernel/vmcore_info: Register dynamic information into meminspect

Register vmcoreinfo information into the inspection table.
Because the size of the info is computed after all entries are
added, there is no point in registering the whole page; instead,
call the inspection registration once everything is in place with
the right size. A second reason is that the vmcoreinfo is added
as a region inside the ELF core image note, so there is no point
in having blank space at the end.

> diff --git a/kernel/vmcore_info.c b/kernel/vmcore_info.c
> index 8614430ca212a..c5181e9d049cf 100644
> --- a/kernel/vmcore_info.c
> +++ b/kernel/vmcore_info.c

[ ... ]

> @@ -247,6 +248,9 @@ static int __init crash_save_vmcoreinfo_init(void)
>  	arch_crash_save_vmcoreinfo();
>  	update_vmcoreinfo_note();
>  
> +	meminspect_lock_register_id_va(MEMINSPECT_ID_VMCOREINFO,
> +				       (void *)vmcoreinfo_data, vmcoreinfo_size);

[Severity: High]
If this registration captures the address and size at initialization time,
does it miss the crash-time updates?

Looking at crash_save_vmcoreinfo(), it appends the crash time which increases
vmcoreinfo_size, and it can conditionally swap the pointer to a safecopy:

kernel/vmcore_info.c:crash_save_vmcoreinfo() {
	...
	if (vmcoreinfo_data_safecopy)
		vmcoreinfo_data = vmcoreinfo_data_safecopy;

	vmcoreinfo_append_str("CRASHTIME=%lld\n", ktime_get_real_seconds());
	...
}

Will the meminspect table retain the stale size and original address,
resulting in truncated or potentially corrupted crash dumps since the safe
buffer redirect is missed?


[Severity: High]
Does this unaligned size cause issues when concatenating meminspect regions?

If vmcoreinfo_size is not 4-byte aligned, registering the exact size here
seems to conflict with how the meminspect ELF header generator calculates
segment offsets:

kernel/meminspect/meminspect.c:init_elfheader() {
	...
	note_len = (... +
	    ALIGN(vmcoreinfo_size, 4));

	buf_sz = elfh_size + note_len - ALIGN(vmcoreinfo_size, 4);
	...
}

Could the missing padding cause the ELF core dump structure to become
misaligned, making the p_offset pointers incorrect and failing to parse
in external tools?

> +
>  	return 0;
>  }

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