From: ebiederm@xmission.com (Eric W. Biederman)
To: Liao Chang <liaochang1@huawei.com>
Cc: paul.walmsley@sifive.com, palmer@dabbelt.com,
aou@eecs.berkeley.edu, mick@ics.forth.gr, jszhang@kernel.org,
guoren@linux.alibaba.com, penberg@kernel.org,
sunnanyong@huawei.com, wangkefeng.wang@huawei.com,
changbin.du@intel.com, alex@ghiti.fr,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
kexec@lists.infradead.org
Subject: Re: [PATCH 2/3] RISC-V: use memcpy for kexec_file mode
Date: Fri, 29 Oct 2021 22:49:09 -0500 [thread overview]
Message-ID: <87ee83goju.fsf@disp2133> (raw)
In-Reply-To: <20211030031832.165457-3-liaochang1@huawei.com> (Liao Chang's message of "Sat, 30 Oct 2021 11:18:31 +0800")
Liao Chang <liaochang1@huawei.com> writes:
> The pointer to buffer loading kernel binaries is in kernel space for
> kexec_fil mode, When copy_from_user copies data from pointer to a block
> of memory, it checkes that the pointer is in the user space range, on
> RISCV-V that is:
>
> static inline bool __access_ok(unsigned long addr, unsigned long size)
> {
> return size <= TASK_SIZE && addr <= TASK_SIZE - size;
> }
>
> and TASK_SIZE is 0x4000000000 for 64-bits, which now causes
> copy_from_user to reject the access of the field 'buf' of struct
> kexec_segment that is in range [CONFIG_PAGE_OFFSET - VMALLOC_SIZE,
> CONFIG_PAGE_OFFSET), is invalid user space pointer.
>
> This patch fixes this issue by skipping access_ok(), use mempcy() instead.
I am a bit confused.
Why is machine_kexec ever calling copy_from_user? That seems wrong in
all cases.
Even worse then having a copy_from_user is having data that you don't
know if you should call copy_from_user on.
There is most definitely a bug here. Can someone please sort it out
without making the kernel guess what kind of memory it is copying from.
Eric
> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> ---
> arch/riscv/kernel/machine_kexec.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/machine_kexec.c b/arch/riscv/kernel/machine_kexec.c
> index e6eca271a4d6..4a5db856919b 100644
> --- a/arch/riscv/kernel/machine_kexec.c
> +++ b/arch/riscv/kernel/machine_kexec.c
> @@ -65,7 +65,9 @@ machine_kexec_prepare(struct kimage *image)
> if (image->segment[i].memsz <= sizeof(fdt))
> continue;
>
> - if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
> + if (image->file_mode)
> + memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
> + else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
> continue;
>
> if (fdt_check_header(&fdt))
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Liao Chang <liaochang1@huawei.com>
Cc: <paul.walmsley@sifive.com>, <palmer@dabbelt.com>,
<aou@eecs.berkeley.edu>, <mick@ics.forth.gr>,
<jszhang@kernel.org>, <guoren@linux.alibaba.com>,
<penberg@kernel.org>, <sunnanyong@huawei.com>,
<wangkefeng.wang@huawei.com>, <changbin.du@intel.com>,
<alex@ghiti.fr>, <linux-riscv@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <kexec@lists.infradead.org>
Subject: Re: [PATCH 2/3] RISC-V: use memcpy for kexec_file mode
Date: Fri, 29 Oct 2021 22:49:09 -0500 [thread overview]
Message-ID: <87ee83goju.fsf@disp2133> (raw)
In-Reply-To: <20211030031832.165457-3-liaochang1@huawei.com> (Liao Chang's message of "Sat, 30 Oct 2021 11:18:31 +0800")
Liao Chang <liaochang1@huawei.com> writes:
> The pointer to buffer loading kernel binaries is in kernel space for
> kexec_fil mode, When copy_from_user copies data from pointer to a block
> of memory, it checkes that the pointer is in the user space range, on
> RISCV-V that is:
>
> static inline bool __access_ok(unsigned long addr, unsigned long size)
> {
> return size <= TASK_SIZE && addr <= TASK_SIZE - size;
> }
>
> and TASK_SIZE is 0x4000000000 for 64-bits, which now causes
> copy_from_user to reject the access of the field 'buf' of struct
> kexec_segment that is in range [CONFIG_PAGE_OFFSET - VMALLOC_SIZE,
> CONFIG_PAGE_OFFSET), is invalid user space pointer.
>
> This patch fixes this issue by skipping access_ok(), use mempcy() instead.
I am a bit confused.
Why is machine_kexec ever calling copy_from_user? That seems wrong in
all cases.
Even worse then having a copy_from_user is having data that you don't
know if you should call copy_from_user on.
There is most definitely a bug here. Can someone please sort it out
without making the kernel guess what kind of memory it is copying from.
Eric
> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> ---
> arch/riscv/kernel/machine_kexec.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/machine_kexec.c b/arch/riscv/kernel/machine_kexec.c
> index e6eca271a4d6..4a5db856919b 100644
> --- a/arch/riscv/kernel/machine_kexec.c
> +++ b/arch/riscv/kernel/machine_kexec.c
> @@ -65,7 +65,9 @@ machine_kexec_prepare(struct kimage *image)
> if (image->segment[i].memsz <= sizeof(fdt))
> continue;
>
> - if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
> + if (image->file_mode)
> + memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
> + else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
> continue;
>
> if (fdt_check_header(&fdt))
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Liao Chang <liaochang1@huawei.com>
Cc: <paul.walmsley@sifive.com>, <palmer@dabbelt.com>,
<aou@eecs.berkeley.edu>, <mick@ics.forth.gr>,
<jszhang@kernel.org>, <guoren@linux.alibaba.com>,
<penberg@kernel.org>, <sunnanyong@huawei.com>,
<wangkefeng.wang@huawei.com>, <changbin.du@intel.com>,
<alex@ghiti.fr>, <linux-riscv@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <kexec@lists.infradead.org>
Subject: Re: [PATCH 2/3] RISC-V: use memcpy for kexec_file mode
Date: Fri, 29 Oct 2021 22:49:09 -0500 [thread overview]
Message-ID: <87ee83goju.fsf@disp2133> (raw)
In-Reply-To: <20211030031832.165457-3-liaochang1@huawei.com> (Liao Chang's message of "Sat, 30 Oct 2021 11:18:31 +0800")
Liao Chang <liaochang1@huawei.com> writes:
> The pointer to buffer loading kernel binaries is in kernel space for
> kexec_fil mode, When copy_from_user copies data from pointer to a block
> of memory, it checkes that the pointer is in the user space range, on
> RISCV-V that is:
>
> static inline bool __access_ok(unsigned long addr, unsigned long size)
> {
> return size <= TASK_SIZE && addr <= TASK_SIZE - size;
> }
>
> and TASK_SIZE is 0x4000000000 for 64-bits, which now causes
> copy_from_user to reject the access of the field 'buf' of struct
> kexec_segment that is in range [CONFIG_PAGE_OFFSET - VMALLOC_SIZE,
> CONFIG_PAGE_OFFSET), is invalid user space pointer.
>
> This patch fixes this issue by skipping access_ok(), use mempcy() instead.
I am a bit confused.
Why is machine_kexec ever calling copy_from_user? That seems wrong in
all cases.
Even worse then having a copy_from_user is having data that you don't
know if you should call copy_from_user on.
There is most definitely a bug here. Can someone please sort it out
without making the kernel guess what kind of memory it is copying from.
Eric
> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> ---
> arch/riscv/kernel/machine_kexec.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/machine_kexec.c b/arch/riscv/kernel/machine_kexec.c
> index e6eca271a4d6..4a5db856919b 100644
> --- a/arch/riscv/kernel/machine_kexec.c
> +++ b/arch/riscv/kernel/machine_kexec.c
> @@ -65,7 +65,9 @@ machine_kexec_prepare(struct kimage *image)
> if (image->segment[i].memsz <= sizeof(fdt))
> continue;
>
> - if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
> + if (image->file_mode)
> + memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
> + else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
> continue;
>
> if (fdt_check_header(&fdt))
next prev parent reply other threads:[~2021-10-30 3:49 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-30 3:18 [PATCH 0/3] riscv: kexec: add kexec_file_load() support Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-10-30 3:18 ` [PATCH 1/3] kexec_file: Fix kexec_file.c build error for riscv platform Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-10-30 3:18 ` [PATCH 2/3] RISC-V: use memcpy for kexec_file mode Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-10-30 3:49 ` Eric W. Biederman [this message]
2021-10-30 3:49 ` Eric W. Biederman
2021-10-30 3:49 ` Eric W. Biederman
2021-10-31 11:14 ` Björn Töpel
2021-10-31 11:14 ` Björn Töpel
2021-10-31 11:14 ` Björn Töpel
2021-11-01 21:15 ` Eric W. Biederman
2021-11-01 21:15 ` Eric W. Biederman
2021-11-01 21:15 ` Eric W. Biederman
2021-11-02 3:52 ` liaochang (A)
2021-11-02 3:52 ` liaochang (A)
2021-11-02 3:52 ` liaochang (A)
2021-10-30 3:18 ` [PATCH 3/3] RISC-V: Add kexec_file support Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-10-30 3:18 ` Liao Chang
2021-11-01 16:36 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ee83goju.fsf@disp2133 \
--to=ebiederm@xmission.com \
--cc=alex@ghiti.fr \
--cc=aou@eecs.berkeley.edu \
--cc=changbin.du@intel.com \
--cc=guoren@linux.alibaba.com \
--cc=jszhang@kernel.org \
--cc=kexec@lists.infradead.org \
--cc=liaochang1@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mick@ics.forth.gr \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=penberg@kernel.org \
--cc=sunnanyong@huawei.com \
--cc=wangkefeng.wang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.