Messages in this thread | | | From | (Eric W. Biederman) | Date | Mon, 01 Nov 2021 16:15:42 -0500 | Subject | Re: [PATCH 2/3] RISC-V: use memcpy for kexec_file mode |
| |
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
| |