Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH] microblaze: remove CONFIG_SET_FS | Date | Thu, 10 Feb 2022 14:21:07 +0000 |
| |
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)
| |