From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752973AbcHQTRr (ORCPT ); Wed, 17 Aug 2016 15:17:47 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:35446 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752545AbcHQTRq (ORCPT ); Wed, 17 Aug 2016 15:17:46 -0400 Date: Wed, 17 Aug 2016 20:15:34 +0100 From: Al Viro To: Rob Clark Cc: Vaishali Thakkar , David Airlie , linux-arm-msm , "dri-devel@lists.freedesktop.org" , freedreno@lists.freedesktop.org, Linux Kernel Mailing List , Julia Lawall Subject: Re: Use of copy_from_user in msm_gem_submit.c while holding a spin_lock Message-ID: <20160817191534.GF2356@ZenIV.linux.org.uk> References: <57B44D2E.2030301@oracle.com> <20160817170827.GC2356@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.