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

[Sourabh Jain <sourabhjain@linux.ibm.com>]

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;


Thanks,
Sourabh Jain