| Message ID | 20190209194718.1294-1-paul.burton@mips.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | MIPS: Fix access_ok() for the last byte of user space | expand |
On Sat, Feb 9, 2019 at 11:47 AM Paul Burton <paul.burton@mips.com> wrote: > > Fix MIPS in the same way as alpha & SH, by subtracting one from the addr > + size condition when size is non-zero. With this the access_ok() calls > above return 1 indicating that the access may be valid. You might want to update the comment above that thing too.. Linus
On 2/9/19 8:47 PM, Paul Burton wrote: > The MIPS implementation of access_ok() incorrectly reports that access > to the final byte of user memory is not OK, much as the alpha & SH > versions did prior to commit 94bd8a05cd4d ("Fix 'acccess_ok()' on alpha > and SH"). > > For example on a MIPS64 system with __UA_LIMIT == 0xffff000000000000 we > incorrectly fail in the following cases: > > access_ok(0xffffffffffff, 0x1) = 0 > access_ok(0xfffffffffffe, 0x2) = 0 > > Fix MIPS in the same way as alpha & SH, by subtracting one from the addr > + size condition when size is non-zero. With this the access_ok() calls > above return 1 indicating that the access may be valid. > > The cost of the improved check is pretty minimal - we gain 2410 bytes, > or 0.03%, in kernel code size for a 64r6el_defconfig kernel built using > GCC 8.1.0. > > Signed-off-by: Paul Burton <paul.burton@mips.com> > Reported-by: Linus Torvalds <torvalds@linux-foundation.org> > --- > > arch/mips/include/asm/uaccess.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h > index d43c1dc6ef15..774c0f955ab0 100644 > --- a/arch/mips/include/asm/uaccess.h > +++ b/arch/mips/include/asm/uaccess.h > @@ -128,7 +128,9 @@ static inline bool eva_kernel_access(void) > static inline int __access_ok(const void __user *p, unsigned long size) > { > unsigned long addr = (unsigned long)p; > - return (get_fs().seg & (addr | (addr + size) | __ua_size(size))) == 0; > + unsigned long end = addr + size - !!size; > + > + return (get_fs().seg & (addr | end | __ua_size(size))) == 0; > } > > #define access_ok(addr, size) \ > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff --git a/arch/mips/include/asm/uaccess.h b/arch/mips/include/asm/uaccess.h index d43c1dc6ef15..774c0f955ab0 100644 --- a/arch/mips/include/asm/uaccess.h +++ b/arch/mips/include/asm/uaccess.h @@ -128,7 +128,9 @@ static inline bool eva_kernel_access(void) static inline int __access_ok(const void __user *p, unsigned long size) { unsigned long addr = (unsigned long)p; - return (get_fs().seg & (addr | (addr + size) | __ua_size(size))) == 0; + unsigned long end = addr + size - !!size; + + return (get_fs().seg & (addr | end | __ua_size(size))) == 0; } #define access_ok(addr, size) \
The MIPS implementation of access_ok() incorrectly reports that access to the final byte of user memory is not OK, much as the alpha & SH versions did prior to commit 94bd8a05cd4d ("Fix 'acccess_ok()' on alpha and SH"). For example on a MIPS64 system with __UA_LIMIT == 0xffff000000000000 we incorrectly fail in the following cases: access_ok(0xffffffffffff, 0x1) = 0 access_ok(0xfffffffffffe, 0x2) = 0 Fix MIPS in the same way as alpha & SH, by subtracting one from the addr + size condition when size is non-zero. With this the access_ok() calls above return 1 indicating that the access may be valid. The cost of the improved check is pretty minimal - we gain 2410 bytes, or 0.03%, in kernel code size for a 64r6el_defconfig kernel built using GCC 8.1.0. Signed-off-by: Paul Burton <paul.burton@mips.com> Reported-by: Linus Torvalds <torvalds@linux-foundation.org> --- arch/mips/include/asm/uaccess.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)