RE: [PATCH] microblaze: remove CONFIG_SET_FS

[David Laight <David.Laight@ACULAB.COM>]

From: Arnd Bergmann
> Sent: 10 February 2022 13:30
...
> #define __access_ok(addr, size) \
>         ((get_fs().seg == KERNEL_DS.seg) || \
>         (((unsigned long)addr < get_fs().seg) && \
>           (unsigned long)size < (get_fs().seg - (unsigned long)addr)))

That one is strange.
Seems to be optimised for kernel accesses!

> ia64 and sparc skip the size check entirely but rely on an unmapped page
> at the beginning of the kernel address range, which avoids this problem
> but may also be dangerous.

An unmapped page requires that the kernel do sequential accesses
(or, at least, not big offset) - which is normally fine.
Especially for 64bit where there is plenty of address space.
I guess it could be problematic for 32bit if you can/want to
use 'big pages' for the kernel addresses.
Losing a single (typically) 4k page isn't a problem.

Certainly not mapping the page at TASK_SIZE is a good safety check.
Actually, setting TASK_SIZE to 0xc0000000 - PAGE_SIZE and never
mapping the last user page has the same effect.
Except I bet the ldso has to go there :-(
Not to mention the instruction sets where loading the constant
would then be two instructions.

...
> > Although typical 64bit architectures can use the faster:
> >         ((addr | size) >> 62)
> 
> I think this is the best version, and it's already widely used:

I just did a quick check, both clang and gcc optimise out
constant values for 'size'.

> static inline int __range_ok(unsigned long addr, unsigned long size)
> {
>         return size <= TASK_SIZE && addr <= (TASK_SIZE - size);
> }
> 
> since 'size' is usually constant, so this turns into a single comparison
> against a compile-time constant.

Hmmm... maybe there should be a comment that it is the same as
the more obvious:
	(addr <= TASK_SIZE && addr <= TASK_SIZE - size)
but is better for constant size.
(Provided TASK_SIZE is a constant.)

I'm sure Linus was 'unhappy' about checking against 2^63 for
32bit processes on a 64bit kernel.

Hmmm compat code that has 32bit addr/size needn't even call
access_ok() - it can never access kernel memory at all.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)