On Wed, Sep 8, 2021 at 11:40 PM Helge Deller <deller@xxxxxx> wrote:
> On 9/8/21 11:26 PM, Arnd Bergmann wrote:
> >> /*
> >> * Complex access routines -- macros
> >> */
> >> -#define user_addr_max() (~0UL)
> >> +#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)
>
> I noticed that our user_addr_max() was actually wrong.
> It's used in the generic strnlen_user() so fixing it seemed appropriate.
The user_addr_max() definition should match what you do in access_ok()
for USER_DS though, which is a nop on architectures that have split user
address space, so I think the ~0UL was correct here.
No matter what the arguments are, there is no danger of spilling over
into kernel space, which is what the user_addr_max() check is trying
to prevent on other architectures.
> > We are getting very close to completely removing set_fs()/get_fs(),
> > uaccess_kernel() and some related bits from the kernel, so I think
> > it would be better to the other way here and finish off removing
> > CONFIG_SET_FS from parisc.
> >
> > I think this will also simplify your asm/uaccess.h a lot, in particular
> > since it has separate address spaces for __get_user() and
> > __get_kernel_nofault(), and without set_fs() you can leave out
> > the runtime conditional to switch between them.
>
> That's a good idea and should probably be done.
> Do you have some pointers where to start, e.g. initial commits from other arches ?
Russell just merged my series for arch/arm in linux-5.15, you
can look at that but it's probably easier for parisc.
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.
Arnd
