[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <187a4850.c0d8.18dd0214ffe.Coremail.gaoshanliukou@163.com>
Date: Thu, 22 Feb 2024 17:24:20 +0800 (CST)
From: "yang.zhang" <gaoshanliukou@....com>
To: "Baoquan He" <bhe@...hat.com>
Cc: ebiederm@...ssion.com, kexec@...ts.infradead.org,
linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
"yang.zhang" <yang.zhang@...intek.com>
Subject: Re:Re: [PATCH v2] kexec: copy only happens before uchunk goes to
zero
At 2024-02-22 15:45:38, "Baoquan He" <bhe@...hat.com> wrote:
>On 02/20/24 at 10:11am, yang.zhang wrote:
>> From: "yang.zhang" <yang.zhang@...intek.com>
>>
>> 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@...hat.com>
Thanks, i update the commits and post v3.>
>>
>> Signed-off-by: yang.zhang <yang.zhang@...intek.com>
>>
>> ---
>> v1 -> v2:
>> - Only copy before uchunk goes to zero
>>
>> V1: https://lore.kernel.org/lkml/20240130101802.23850-1-gaoshanliukou@163.com/
>> ---
>> 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
>>
Powered by blists - more mailing lists