From: ebiederm@xmission.com (Eric W. Biederman)
To: "Björn Töpel" <bjorn.topel@gmail.com>
Cc: Liao Chang <liaochang1@huawei.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Nick Kossifidis <mick@ics.forth.gr>,
jszhang@kernel.org, guoren@linux.alibaba.com,
Pekka Enberg <penberg@kernel.org>,
sunnanyong@huawei.com, wangkefeng.wang@huawei.com,
changbin.du@intel.com, Alex Ghiti <alex@ghiti.fr>,
linux-riscv <linux-riscv@lists.infradead.org>,
LKML <linux-kernel@vger.kernel.org>,
kexec@lists.infradead.org
Subject: Re: [PATCH 2/3] RISC-V: use memcpy for kexec_file mode
Date: Mon, 01 Nov 2021 16:15:42 -0500 [thread overview]
Message-ID: <87a6inbmrl.fsf@disp2133> (raw)
In-Reply-To: <CAJ+HfNjaBQrBtOuMvTccbb2K-Ua=T1eZk0+70hp0_aOAc83A-Q@mail.gmail.com> ("Björn Töpel"'s message of "Sun, 31 Oct 2021 12:14:33 +0100")
Björn Töpel <bjorn.topel@gmail.com> writes:
> On Sat, 30 Oct 2021 at 05:51, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>
>> 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.
>>
>
> It's not machine_kexec -- it's machine_kexec_prepare, which pulls out
> the FDT from the image. It looks like MIPS does it similarly.
>
> (Caveat: I might be confused as well! ;-))
True it is machine_kexec_prepare. But still. copy_from_user does not
belong in there. It is not passed a userspace pointer.
This looks more like a case for kmap to read a table from the firmware.
Even if it someone made sense it definitely does not make sense to
make it a conditional copy_from_user. That way lies madness.
The entire change is a smell that there is some abstraction that is
going wrong, and that abstraction needs to get fixed.
Eric
_______________________________________________
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: "Björn Töpel" <bjorn.topel@gmail.com>
Cc: Liao Chang <liaochang1@huawei.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Nick Kossifidis <mick@ics.forth.gr>,
jszhang@kernel.org, guoren@linux.alibaba.com,
Pekka Enberg <penberg@kernel.org>,
sunnanyong@huawei.com, wangkefeng.wang@huawei.com,
changbin.du@intel.com, Alex Ghiti <alex@ghiti.fr>,
linux-riscv <linux-riscv@lists.infradead.org>,
LKML <linux-kernel@vger.kernel.org>,
kexec@lists.infradead.org
Subject: Re: [PATCH 2/3] RISC-V: use memcpy for kexec_file mode
Date: Mon, 01 Nov 2021 16:15:42 -0500 [thread overview]
Message-ID: <87a6inbmrl.fsf@disp2133> (raw)
In-Reply-To: <CAJ+HfNjaBQrBtOuMvTccbb2K-Ua=T1eZk0+70hp0_aOAc83A-Q@mail.gmail.com> ("Björn Töpel"'s message of "Sun, 31 Oct 2021 12:14:33 +0100")
Björn Töpel <bjorn.topel@gmail.com> writes:
> On Sat, 30 Oct 2021 at 05:51, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>
>> 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.
>>
>
> It's not machine_kexec -- it's machine_kexec_prepare, which pulls out
> the FDT from the image. It looks like MIPS does it similarly.
>
> (Caveat: I might be confused as well! ;-))
True it is machine_kexec_prepare. But still. copy_from_user does not
belong in there. It is not passed a userspace pointer.
This looks more like a case for kmap to read a table from the firmware.
Even if it someone made sense it definitely does not make sense to
make it a conditional copy_from_user. That way lies madness.
The entire change is a smell that there is some abstraction that is
going wrong, and that abstraction needs to get fixed.
Eric
_______________________________________________
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: "Björn Töpel" <bjorn.topel@gmail.com>
Cc: Liao Chang <liaochang1@huawei.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Nick Kossifidis <mick@ics.forth.gr>,
jszhang@kernel.org, guoren@linux.alibaba.com,
Pekka Enberg <penberg@kernel.org>,
sunnanyong@huawei.com, wangkefeng.wang@huawei.com,
changbin.du@intel.com, Alex Ghiti <alex@ghiti.fr>,
linux-riscv <linux-riscv@lists.infradead.org>,
LKML <linux-kernel@vger.kernel.org>,
kexec@lists.infradead.org
Subject: Re: [PATCH 2/3] RISC-V: use memcpy for kexec_file mode
Date: Mon, 01 Nov 2021 16:15:42 -0500 [thread overview]
Message-ID: <87a6inbmrl.fsf@disp2133> (raw)
In-Reply-To: <CAJ+HfNjaBQrBtOuMvTccbb2K-Ua=T1eZk0+70hp0_aOAc83A-Q@mail.gmail.com> ("Björn Töpel"'s message of "Sun, 31 Oct 2021 12:14:33 +0100")
Björn Töpel <bjorn.topel@gmail.com> writes:
> On Sat, 30 Oct 2021 at 05:51, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>
>> 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.
>>
>
> It's not machine_kexec -- it's machine_kexec_prepare, which pulls out
> the FDT from the image. It looks like MIPS does it similarly.
>
> (Caveat: I might be confused as well! ;-))
True it is machine_kexec_prepare. But still. copy_from_user does not
belong in there. It is not passed a userspace pointer.
This looks more like a case for kmap to read a table from the firmware.
Even if it someone made sense it definitely does not make sense to
make it a conditional copy_from_user. That way lies madness.
The entire change is a smell that there is some abstraction that is
going wrong, and that abstraction needs to get fixed.
Eric
next prev parent reply other threads:[~2021-11-01 21:15 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
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 [this message]
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=87a6inbmrl.fsf@disp2133 \
--to=ebiederm@xmission.com \
--cc=alex@ghiti.fr \
--cc=aou@eecs.berkeley.edu \
--cc=bjorn.topel@gmail.com \
--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.