lkml.org 
[lkml]   [2020]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
On Thu, May 14, 2020 at 03:45:09PM +0200, Bartlomiej Zolnierkiewicz wrote:
>
> Hi Al,
>
> On 5/10/20 1:45 AM, Al Viro wrote:
> > From: Al Viro <viro@zeniv.linux.org.uk>
> >
> > addresses passed only to get_user() and put_user()
>
> This driver lacks checks for {get,put}_user() return values so it will
> now return 0 ("success") even if {get,put}_user() fails.
>
> Am I missing something?

"now" is interesting, considering
/* We let the MMU do all checking */
static inline int access_ok(const void __user *addr,
unsigned long size)
{
return 1;
}
in arch/m68k/include/asm/uaccess_mm.h

Again, access_ok() is *NOT* about checking if memory is readable/writable/there
in the first place. All it does is a static check that address is in
"userland" range - on architectures that have kernel and userland sharing the
address space. On architectures where we have separate ASI or equivalents
thereof for kernel and for userland the fscker is always true.

If MMU will prevent access to kernel memory by uaccess insns for given address
range, access_ok() is fine with it. It does not do anything else.

And yes, get_user()/put_user() callers should handle the fact that those can
fail. Which they bloody well can _after_ _success_ of access_ok(). And
without any races whatsoever.

IOW, the lack of such checks is a bug, but it's quite independent from the
bogus access_ok() call. On any architecture. mmap() something, munmap()
it and pass the address where it used to be to that ioctl(). Failing
get_user()/put_user() is guaranteed, so's succeeding access_ok().

And that code is built only on amiga, so access_ok() always succeeds, anyway.

\
 
 \ /
  Last update: 2020-05-14 16:07    [W:0.104 / U:0.728 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site