Re: [PATCH v1 1/2] kho: check scratch vs CMA alignment at runtime

[Pratyush Yadav <pratyush@kernel.org>]

On Thu, Oct 01 2026, Sourabh Jain wrote:

> On 28/09/26 14:11, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] kho: bypassed scratch alignment check during resume causes memory corruption
>> --
>>
>> commit cbd7cb7f008e704b389d8dfe3306e1e83e49e8a8
>> Author: Sourabh Jain <sourabhjain@linux.ibm.com>
>>
>> kho: check scratch vs CMA alignment at runtime
>>
>> KHO requires the scratch alignment to be at least CMA_MIN_ALIGNMENT_BYTES,
>> which was previously enforced with a static_assert(). This commit replaces
>> the static_assert() with a runtime check in kho_reserve_scratch() to allow
>> KHO on configurations where the minimum alignment is not a constant.
>>
>>> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
>>> index 7c4d86daf86d5..dc76dd6b74da7 100644
>>> --- a/kernel/liveupdate/kexec_handover.c
>>> +++ b/kernel/liveupdate/kexec_handover.c
>> [ ... ]
>>
>>> @@ -836,6 +828,21 @@ static void __init kho_reserve_scratch(void)
>>>   	if (!kho_enable)
>>>   		return;
>>>   +	/*
>>> +	 * This is the minimal alignment required by deferred struct page init.
>>> +	 * deferred_init_memmap_chunk frees memory to the buddy allocator, which looks
>>> +	 * at the neighboring pages (up to MAX_PAGE_ORDER) to merge them.
>>> +	 * If KHO scratch is not aligned to that value, buddy can access uninitialized
>>> +	 * struct pages, which can cause a crash.
>>> +	 */
>>> +	if (WARN_ON_ONCE(SCRATCH_ALIGNMENT_BYTES < CMA_MIN_ALIGNMENT_BYTES)) {
>>> +		pr_err("scratch alignment %lu < CMA minimum %lu, disabling\n",
>>> +		       (unsigned long)SCRATCH_ALIGNMENT_BYTES,
>>> +		       (unsigned long)CMA_MIN_ALIGNMENT_BYTES);
>>> +		kho_enable = false;
>>> +		return;
>>> +	}
>>> +
>> [Severity: High]
>> Does this new runtime check get bypassed during a KHO resume?
>>
>> During a KHO resume, kho_memory_init() skips kho_reserve_scratch():
>>
>> kernel/liveupdate/kexec_handover.c:kho_memory_init() {
>> 	if (kho_in.scratch_phys)
>> 		kho_mem_retrieve();
>> 	else
>> 		kho_reserve_scratch();
>> }
>>
>> If a predecessor kernel passes a scratch region with a smaller alignment,
>> and the successor kernel has a larger pageblock_order, bypassing this check
>> means kho_init() will iterate over the provided scratch memory using the
>> successor kernel's larger pageblock_nr_pages:
>>
>> kernel/liveupdate/kexec_handover.c:kho_init() {
>> 		for (pfn = base_pfn; pfn < base_pfn + count;
>> 		     pfn += pageblock_nr_pages)
>> 			init_cma_reserved_pageblock(pfn_to_page(pfn));
>> }
>> Can this lead to init_cma_reserved_pageblock() overwriting state for pages
>> outside the actual scratch region bounds, or cause a crash in the buddy
>> allocator if __free_pages() is called on an unaligned PFN?
>
> Yes, if the kexeced kernel has a higher pageblock_order than the
> kernel that initiated the kexec with KHO, this can cause problems when
> the code above hands scratch-memory pages back to the buddy allocator
> in the kexeced kernel.
>
> Would it make sense to handle this by keeping only the unaligned pages
> reserved and handing the remaining pages back to the buddy?
>
> Something like this:
>
> diff --git a/kernel/liveupdate/kexec_handover.c
> b/kernel/liveupdate/kexec_handover.c
> index 2e3a36054851..dea6e7790972 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -1928,7 +1928,8 @@ static __init int kho_init(void)
>
>         for (int i = 0; i < kho_scratch_cnt; i++) {
>                 unsigned long base_pfn = PHYS_PFN(kho_scratch[i].addr);
> -               unsigned long count = kho_scratch[i].size >> PAGE_SHIFT;
> +              unsigned long count = ALIGN_DOWN(kho_scratch[i].size >>
> PAGE_SHIFT,
> + pageblock_nr_pages);
>                 unsigned long pfn;

This code only runs on cold boot. So it will never run for kexec-ed
kernel. See the if (fdt) check above. It is very subtle and easy to
miss.

For kexeced kernels, the migrate type is set via
kho_scratch_migratetype() (see its callers). The logic it uses is that
if any page in the whole page block overlaps with scratch, mark it as
MIGRATE_CMA. So I think it already works with variable pageblock sizes.

>

Also, off-topic nit: your email client seems to like sending
"non-breaking space" (0xC2 0xA0 in UTF-8). For example, for indentation
in the diff above, it uses non-breaking space, and makes the diff hard
to apply. Perhaps tinker with the settings and see if you can avoid it?

-- 
Regards,
Pratyush Yadav