在 2023/12/13 12:44, Baoquan He 写道:
On 11/30/23 at 09:20pm, fuqiang wang wrote:
On 2023/11/30 15:44, Baoquan He wrote:
On 11/27/23 at 10:56am, fuqiang wang wrote:
When the split happened, judge whether mem->nr_ranges is equal to
mem->max_nr_ranges. If it is true, return -ENOMEM.
The advantage of doing this is that it can avoid array bounds caused by
some bugs. E.g., Before commit 4831be702b95 ("arm64/kexec: Fix missing
extra range for crashkres_low."), reserve both high and low memories for
the crashkernel may cause out of bounds.
On the other hand, move this code before the split to ensure that the
array will not be changed when return error.
If out of array boundary is caused, means the laoding failed, whether
the out of boundary happened or not. I don't see how this code change
makes sense. Do I miss anything?
Thanks
Baoquan
Hi baoquan,
In some configurations, out of bounds may not cause crash_exclude_mem_range()
returns error, then the load will succeed.
E.g.
There is a cmem before execute crash_exclude_mem_range():
cmem = {
max_nr_ranges = 3
nr_ranges = 2
ranges = {
{start = 1, end = 1000}
{start = 1001, end = 2000}
}
}
After executing twice crash_exclude_mem_range() with the start/end params
100/200, 300/400 respectively, the cmem will be:
cmem = {
max_nr_ranges = 3
nr_ranges = 4 <== nr_ranges > max_nr_ranges
ranges = {
{start = 1, end = 99 }
{start = 201, end = 299 }
{start = 401, end = 1000}
{start = 1001, end = 2000} <== OUT OF BOUNDS
}
}
When an out of bounds occurs during the second execution, the function will not
return error.
Additionally, when the function returns error, means the load failed. It seems
meaningless to keep the original data unchanged. But in my opinion, this will
make this function more rigorous and more versatile. (However, I am not sure if
it is self-defeating and I hope to receive more suggestions).
Sorry for late reply.
I checked the code again, there seems to be cases out of bounds occur
very possiblly. We may need to enlarge the cmem array to avoid the risk.
In below draft code, we need add another slot to exclude the low 1M area
when preparing elfcorehdr. And to exclude the elf header region from
crash kernel region, we need create the cmem with 2 slots.
With these change, we can absolutely avoid out of bounds occurence.
What do you think?
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 1715e5f06a59..21facabcf699 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -147,10 +147,10 @@ static struct crash_mem *fill_up_crash_elf_data(void)
return NULL;
/*
- * Exclusion of crash region and/or crashk_low_res may cause
- * another range split. So add extra two slots here.
+ * Exclusion of low 1M, crash region and/or crashk_low_res may
+ * cause another range split. So add extra two slots here.
*/
- nr_ranges += 2;
+ nr_ranges += 3;
cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
if (!cmem)
return NULL;
Hi baoquan,
Exclusion of low 1M may not cause new region. Because when calling
crash_exclude_mem_range(), the start parameter is 0 and the condition for
splitting a new region is that the start, end parameters are both in a certain
existing region in cmem and cannot be equal to existing region's start or end.
Obviously, start (0) cannot meet this condition.
@@ -282,7 +282,7 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
struct crash_memmap_data cmd;
struct crash_mem *cmem;
- cmem = vzalloc(struct_size(cmem, ranges, 1));
+ cmem = vzalloc(struct_size(cmem, ranges, 2));
if (!cmem)
return -ENOMEM;
Yes, you are right. Exclude the elf header region from crash kernel region may
cause split a new region. And there seems to be another issue with this code
path: Before calling crash_exclude_mem_range(), cmem->max_nr_ranges was not
initialized.
In my opinion, these change can absolutely avoid out of bounds occurence. But
when we forget to modify max_nr_ranges due to a mistakes in the future, is it
better to report it by returning an error through crash_exclude_mem_range().
What do you think about it?
Thanks
fuqiang
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
