From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3F782C47257 for ; Thu, 7 May 2020 05:12:19 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D06F220838 for ; Thu, 7 May 2020 05:12:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D06F220838 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lst.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F3EC7900003; Thu, 7 May 2020 01:12:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EC862900002; Thu, 7 May 2020 01:12:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D6A22900003; Thu, 7 May 2020 01:12:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0135.hostedemail.com [216.40.44.135]) by kanga.kvack.org (Postfix) with ESMTP id BB673900002 for ; Thu, 7 May 2020 01:12:17 -0400 (EDT) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 7AAFA181AEF09 for ; Thu, 7 May 2020 05:12:17 +0000 (UTC) X-FDA: 76788751914.15.sink89_310c21817665f X-HE-Tag: sink89_310c21817665f X-Filterd-Recvd-Size: 4209 Received: from verein.lst.de (verein.lst.de [213.95.11.211]) by imf28.hostedemail.com (Postfix) with ESMTP for ; Thu, 7 May 2020 05:12:16 +0000 (UTC) Received: by verein.lst.de (Postfix, from userid 2407) id 990EF68B05; Thu, 7 May 2020 07:12:13 +0200 (CEST) Date: Thu, 7 May 2020 07:12:13 +0200 From: Christoph Hellwig To: Linus Torvalds Cc: Christoph Hellwig , the arch/x86 maintainers , Alexei Starovoitov , Daniel Borkmann , Masami Hiramatsu , Andrew Morton , linux-parisc@vger.kernel.org, linux-um , Netdev , bpf@vger.kernel.org, Linux-MM , Linux Kernel Mailing List Subject: Re: [PATCH 15/15] x86: use non-set_fs based maccess routines Message-ID: <20200507051213.GB4501@lst.de> References: <20200506062223.30032-1-hch@lst.de> <20200506062223.30032-16-hch@lst.de> <20200506181543.GA7873@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, May 06, 2020 at 12:01:32PM -0700, Linus Torvalds wrote: > Oh, absolutely. I did *NOT* mean that you'd use "unsafe_get_user()" as > the actual interface. I just meant that as an implementation detail on > x86, using "unsafe_get_user()" instead of "__get_user_size()" > internally both simplifies the implementation, and means that it > doesn't clash horribly with my local changes. I had a version that just wrapped them, but somehow wasn't able to make it work due to all the side effects vs macros issues. Maybe I need to try again, the current version seemed like a nice way out as it avoided a lot of the silly casting. > Btw, that brings up another issue: so that people can't mis-use those > kernel accessors and use them for user addresses, they probably should > actually do something like > > if ((long)addr >= 0) > goto error_label; > > on x86. IOW, have the "strict" kernel pointer behavior. > > Otherwise somebody will start using them for user pointers, and it > will happen to work on old x86 without CLAC/STAC support. > > Of course, maybe CLAC/STAC is so common these days (at least with > developers) that we don't have to worry about it. The actual public routines (probe_kernel_read and co) get these checks through probe_kernel_read_allowed, which is implemented by the x86 code. Doing this for every 1-8 byte access might be a little slow, though. Do you really fear drivers starting to use the low-level helper? Maybe we need to move those into a different header than that makes it more clear that they are internal? > But here you see what it is, if you want to. __get_user_size() > technically still exists, but it has the "target branch" semantics in > here, so your patch clashes badly with it. The target branch semantics actually are what I want, that is how the maccess code is structured. This is the diff I'd need for the calling conventions in your bundle: diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 765e18417b3ba..d1c8aacedade1 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -526,14 +526,8 @@ do { \ #define HAVE_ARCH_PROBE_KERNEL #define arch_kernel_read(dst, src, type, err_label) \ -do { \ - int __kr_err; \ - \ __get_user_size(*((type *)dst), (__force type __user *)src, \ - sizeof(type), __kr_err); \ - if (unlikely(__kr_err)) \ - goto err_label; \ -} while (0) + sizeof(type), err_label); \ #define arch_kernel_write(dst, src, type, err_label) \ __put_user_size(*((type *)(src)), (__force type __user *)(dst), \