At 2024-02-22 15:45:38, "Baoquan He" <bhe@xxxxxxxxxx> wrote: >On 02/20/24 at 10:11am, yang.zhang wrote: >> From: "yang.zhang" <yang.zhang@xxxxxxxxxxxx> >> >> When load segments, all of the copying and the rest >> only happens before uchunk goes to zero. > >The code change looks good, while the log can be improved with more >details so that people can more easily get what's wrong and fixed. > >=== >When loading segments, ubytes is <= mbytes. When ubytes is exhausted, >there could be remaining mbytes. Then in the while loop, the buf pointer >advancing with mchunk will causing meaningless reading even though it >doesn't harm. > >So let's change to make sure that all of the copying and the rest only >happens before uchunk goes to zero. >=== > >Please take above words as reference to udpate patch log and post v3, >and you can add: > >Acked-by: Baoquan He <bhe@xxxxxxxxxx> Thanks, i update the commits and post v3.> >> >> Signed-off-by: yang.zhang <yang.zhang@xxxxxxxxxxxx> >> >> --- >> v1 -> v2: >> - Only copy before uchunk goes to zero >> >> V1: https://lore.kernel.org/lkml/20240130101802.23850-1-gaoshanliukou@xxxxxxx/ >> --- >> kernel/kexec_core.c | 44 ++++++++++++++++++++++++-------------------- >> 1 file changed, 24 insertions(+), 20 deletions(-) >> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c >> index d08fc7b5db97..2fc3d0e3715a 100644 >> --- a/kernel/kexec_core.c >> +++ b/kernel/kexec_core.c >> @@ -800,22 +800,24 @@ static int kimage_load_normal_segment(struct kimage *image, >> PAGE_SIZE - (maddr & ~PAGE_MASK)); >> uchunk = min(ubytes, mchunk); >> >> - /* For file based kexec, source pages are in kernel memory */ >> - if (image->file_mode) >> - memcpy(ptr, kbuf, uchunk); >> - else >> - result = copy_from_user(ptr, buf, uchunk); >> + if (uchunk) { >> + /* For file based kexec, source pages are in kernel memory */ >> + if (image->file_mode) >> + memcpy(ptr, kbuf, uchunk); >> + else >> + result = copy_from_user(ptr, buf, uchunk); >> + ubytes -= uchunk; >> + if (image->file_mode) >> + kbuf += uchunk; >> + else >> + buf += uchunk; >> + } >> kunmap_local(ptr); >> if (result) { >> result = -EFAULT; >> goto out; >> } >> - ubytes -= uchunk; >> maddr += mchunk; >> - if (image->file_mode) >> - kbuf += mchunk; >> - else >> - buf += mchunk; >> mbytes -= mchunk; >> >> cond_resched(); >> @@ -866,11 +868,18 @@ static int kimage_load_crash_segment(struct kimage *image, >> memset(ptr + uchunk, 0, mchunk - uchunk); >> } >> >> - /* For file based kexec, source pages are in kernel memory */ >> - if (image->file_mode) >> - memcpy(ptr, kbuf, uchunk); >> - else >> - result = copy_from_user(ptr, buf, uchunk); >> + if (uchunk) { >> + /* For file based kexec, source pages are in kernel memory */ >> + if (image->file_mode) >> + memcpy(ptr, kbuf, uchunk); >> + else >> + result = copy_from_user(ptr, buf, uchunk); >> + ubytes -= uchunk; >> + if (image->file_mode) >> + kbuf += uchunk; >> + else >> + buf += uchunk; >> + } >> kexec_flush_icache_page(page); >> kunmap_local(ptr); >> arch_kexec_pre_free_pages(page_address(page), 1); >> @@ -878,12 +887,7 @@ static int kimage_load_crash_segment(struct kimage *image, >> result = -EFAULT; >> goto out; >> } >> - ubytes -= uchunk; >> maddr += mchunk; >> - if (image->file_mode) >> - kbuf += mchunk; >> - else >> - buf += mchunk; >> mbytes -= mchunk; >> >> cond_resched(); >> -- >> 2.25.1 >> _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
