Re: Use of copy_from_user in msm_gem_submit.c while holding a spin_lock

[Al Viro <viro@ZenIV.linux.org.uk>]

On Wed, Aug 17, 2016 at 02:49:32PM -0400, Rob Clark wrote:

> I'm not saying that I shouldn't fix it (although not quite sure how
> yet.. taking/dropping the spinlock inside the loop is not a good
> option from a performance standpoint).  What I am saying is that this
> is not something that can happen accidentally (as it could in the case
> of swap).  But I agree that I should fix it somehow to avoid issues
> with an intentionally evil userspace.

I wouldn't count on that not happening by accident.  With zero changes
in mesa itself - it can be as simple as change of allocator in the
bowels of libc or throwing libdmalloc into the link flags, etc.  And most
of the time it would've worked just fine, but the same call in a situation
when most of the memory is occupied by dirty pagecache pages can end up
having to wait for writeback.

> If there is a copy_from_user() variant that will return an error
> instead of blocking, I think that is really what I want so I can
> implement a slow-path that drops the spin-lock temporarily.

*shrug*

pagefault_disable()/pagefault_enable() are there for purpose, so's
__copy_from_user_inatomic()...  Just remember that __copy_from_user_inatomic()
does not check if the addresses are userland ones (i.e. the caller needs
to check access_ok() itself) and it is *NOT* guaranteed to zero what it
hadn't copied over.  Currently it does zero tail on some, but not all
architectures; come next cycle it and it will not do that zeroing on any
of those.