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

[Rob Clark]

On Wed, Aug 17, 2016 at 3:15 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> 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.

hmm, looks like, at least on arm (not sure about arm64),

#define __copy_from_user_inatomic __copy_from_user

ie. copy_from_user() minus the access_ok() and memset in the
!access_ok() path.. but maybe what I want is just the
pagefault_disable() if that disables copy_from_user() being able to
block..

I guess I need to write evil_test_code.c and see what happens..

BR,
-R