Re: [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 9, 2021 at 4:03 AM Helge Deller <deller@xxxxxx> wrote:
> * Arnd Bergmann <arnd@xxxxxxxxxx>:
> >
> > I think the only part you need to add is __get_kernel_nofault()
> > and __put_kernel_nofault(). You can see in mm/maccess.c
> > what the difference between the two versions in there is.
> >
> > Once you have those, you define HAVE_GET_KERNEL_NOFAULT
> > and then remove CONFIG_SET_FS, set_fs(), get_fs(), load_sr2(),
> > thread_info->addr_limit, KERNEL_DS, and USER_DS.
>
> Thanks for those hints!
> The attached initial patch below seems to work, it boots into systemd.
> It needs further cleanups though.
>
> I wonder if you should add a workaround for 32-bit kernels
> to not copy 64bit-wise, see my patch below in
> copy_from_kernel_nofault():
>
> -       copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
> +       /* avoid 64-bit accesses on 32-bit platforms */
> +       if (sizeof(unsigned long) > 4)
> +               copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);

I would assume that the 8-byte copy would just turn into two 4-byte
accesses here, with exactly the same behavior. Note that this function
changed in 5.15 when I added support for architectures without
unaligned access. You need to include 132b548af559 ("mm/maccess:
fix unaligned copy_{from,to}_kernel_nofault") here for parsic as well,
I assume.

If you prefer to avoid the 8-byte access, this could be done in the
'align = (unsigned long)dst | (unsigned long)src;' statement by
masking out that bit.

> [PATCH] parisc: Implement __get/put_kernel_nofault()
>
> Add __get_kernel_nofault() and __put_kernel_nofault(), define
> HAVE_GET_KERNEL_NOFAULT and remove CONFIG_SET_FS, set_fs(), get_fs(),
> load_sr2(), thread_info->addr_limit, KERNEL_DS, and USER_DS.
>
> Signed-off-by: Helge Deller <deller@xxxxxx>
> Suggested-by: Arnd Bergmann <arnd@xxxxxxxxxx>

Looks good to me overall, with the missing cleanups added.

You should probably include a TASK_SIZE_MAX definition that matches the
old user_addr_max() to avoid looking up the per-task value every time.

       Arnd



[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux