Re: [PATCH v3 01/17] kexec: Record allocated CMA pages to fix release size mismatch

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/9/2 4:14, Mike Rapoport 写道:
> Hi,
> 
>> The CMA pages allocated for a kexec segment are released using the
>> segment's memsz to calculate the number of pages. However, some
>> architecture loaders modify the segment's memsz after allocation
>> (e.g. arm64 subtracts text_offset), causing the release function to
>> free fewer pages than were originally allocated, leaking the remaining
>> CMA pages.
>>
>> Add a per-segment `segment_cma_pages` array to store the number of
>> pages actually allocated from CMA. Populate it during
>> kexec_add_buffer() using the aligned memsz, and use it in
>> kimage_free_cma() to accurately release all allocated pages.
>>
>> This avoids relying on the potentially modified segment->memsz and
>> prevents silent CMA memory leaks.
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Baoquan He <baoquan.he@linux.dev>
>> Cc: Mike Rapoport <rppt@kernel.org>
>> Cc: Pasha Tatashin <pasha.tatashin@soleen.com>
>> Cc: Pratyush Yadav <pratyush@kernel.org>
>> Cc: Brian Mak <makb@juniper.net>
>> Cc: Pingfan Liu <piliu@redhat.com>
>> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Cc: Justinien Bouron <jbouron@amazon.com>
>> Cc: Li Chen <me@linux.beauty>
>> Cc: stable@vger.kernel.org
>> Link: https://sashiko.dev/#/patchset/20260729031235.2840255-1-ruanjinjie%40huawei.com
>> Fixes: 07d24902977e ("kexec: enable CMA based contiguous allocation")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 0af8ae4fdd087..83c296c0eb6cc 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -349,6 +349,7 @@ struct kimage {
>>  	unsigned long nr_segments;
>>  	struct kexec_segment segment[KEXEC_SEGMENT_MAX];
>>  	struct page *segment_cma[KEXEC_SEGMENT_MAX];
>> +	unsigned int segment_cma_pages[KEXEC_SEGMENT_MAX];
> 
> Can we universally use unsigned long for number of pages?

Hi Mike,

unsigned int is used here because the second parameter of
arch_kexec_pre_free_pages is unsigned int.

static inline void arch_kexec_pre_free_pages(void *vaddr, unsigned int
pages) { }

> 
>>  
>>  	struct list_head control_pages;
>>  	struct list_head dest_pages;
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index dc770b9a6d053..611b15bb1369e 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -560,7 +560,7 @@ static void kimage_free_cma(struct kimage *image)
>>  
>>  	for (i = 0; i < image->nr_segments; i++) {
>>  		struct page *cma = image->segment_cma[i];
>> -		u32 nr_pages = image->segment[i].memsz >> PAGE_SHIFT;
>> +		unsigned int nr_pages = image->segment_cma_pages[i];
>>  
>>  		if (!cma)
>>  			continue;
>> @@ -568,6 +568,7 @@ static void kimage_free_cma(struct kimage *image)
>>  		arch_kexec_pre_free_pages(page_address(cma), nr_pages);
>>  		dma_release_from_contiguous(NULL, cma, nr_pages);
>>  		image->segment_cma[i] = NULL;
>> +		image->segment_cma_pages[i] = 0;
>>  	}
>>  
>>  }
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 59fb9d71e9d86..bfae3fee7f2f9 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -670,7 +670,7 @@ static int kexec_walk_resources(struct kexec_buf *kbuf,
>>  
>>  static int kexec_alloc_contig(struct kexec_buf *kbuf)
>>  {
>> -	size_t nr_pages = kbuf->memsz >> PAGE_SHIFT;
>> +	size_t nr_pages = PFN_DOWN(kbuf->memsz);
>>  	unsigned long mem;
>>  	struct page *p;
>>  
>> @@ -756,6 +756,8 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
>>   */
>>  int kexec_add_buffer(struct kexec_buf *kbuf)
>>  {
>> +	unsigned long nr_segments = kbuf->image->nr_segments;
>> +	size_t nr_pages;
>>  	struct kexec_segment *ksegment;
>>  	int ret;
>>  
>> @@ -763,7 +765,7 @@ int kexec_add_buffer(struct kexec_buf *kbuf)
>>  	if (!kbuf->image->file_mode)
>>  		return -EINVAL;
>>  
>> -	if (kbuf->image->nr_segments >= KEXEC_SEGMENT_MAX)
>> +	if (nr_segments >= KEXEC_SEGMENT_MAX)
>>  		return -EINVAL;
>>  
>>  	/*
>> @@ -789,12 +791,18 @@ int kexec_add_buffer(struct kexec_buf *kbuf)
>>  		return ret;
>>  
>>  	/* Found a suitable memory range */
>> -	ksegment = &kbuf->image->segment[kbuf->image->nr_segments];
>> +	ksegment = &kbuf->image->segment[nr_segments];
>>  	ksegment->kbuf = kbuf->buffer;
>>  	ksegment->bufsz = kbuf->bufsz;
>>  	ksegment->mem = kbuf->mem;
>>  	ksegment->memsz = kbuf->memsz;
>> -	kbuf->image->segment_cma[kbuf->image->nr_segments] = kbuf->cma;
>> +	kbuf->image->segment_cma[nr_segments] = kbuf->cma;
>> +	if (kbuf->cma) {
>> +		nr_pages = (unsigned int)(PFN_DOWN(kbuf->memsz));
>> +		kbuf->image->segment_cma_pages[nr_segments] = nr_pages;
> 		kbuf->image->segment_cma_pages[nr_segments] = nr_pages;
>> +	} else {
>> +		kbuf->image->segment_cma_pages[nr_segments] = 0;
>> +	}
> 
> I suggest to rename nr_pages to nr_cma_pages, initialize it to 0 at
> declaration time and make this

I agree with this.

> 
> 	if (kbuf->cma)
> 		nr_cma_pages = PFN_DOWN(kbuf->memsz);
> 	kbuf->image->segment_cma_pages[nr_segments] = nr_cma_pages;
>