From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752117AbdBCT2w (ORCPT ); Fri, 3 Feb 2017 14:28:52 -0500 Received: from mail-oi0-f67.google.com ([209.85.218.67]:32802 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752070AbdBCT2t (ORCPT ); Fri, 3 Feb 2017 14:28:49 -0500 MIME-Version: 1.0 In-Reply-To: <20170203190816.GK27291@ZenIV.linux.org.uk> References: <20170124212327.14517-1-jlayton@redhat.com> <20170125133205.21704-1-jlayton@redhat.com> <20170202095125.GF27291@ZenIV.linux.org.uk> <20170202105651.GA32111@infradead.org> <20170202111625.GG27291@ZenIV.linux.org.uk> <1486040452.2812.6.camel@redhat.com> <20170203072952.GI27291@ZenIV.linux.org.uk> <20170203190816.GK27291@ZenIV.linux.org.uk> From: Linus Torvalds Date: Fri, 3 Feb 2017 11:28:48 -0800 X-Google-Sender-Auth: o30nRwOjRTHD7mIc5Yoqk-ogAAY Message-ID: Subject: Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call To: Al Viro , Steve Capper , Catalin Marinas , Hugh Dickins , Peter Zijlstra , Ingo Molnar Cc: Jeff Layton , Christoph Hellwig , linux-fsdevel , Linux Kernel Mailing List , Linux NFS Mailing List , ceph-devel , lustre-devel@lists.lustre.org, V9FS Developers , Jan Kara , Chris Wilson , "Kirill A. Shutemov" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 3, 2017 at 11:08 AM, Al Viro wrote: > > On x86 it does. I don't see anything equivalent in mm/gup.c one, and the > only kinda-sorta similar thing (access_ok() in __get_user_pages_fast() > there) is vulnerable to e.g. access via kernel_write(). Yeah, access_ok() is bogus. It needs to just check against TASK_SIZE or whatever. > doesn't look promising - access_ok() is never sufficient. Something like > _PAGE_USER tests in x86 one solves that problem, but if anything similar > works for HAVE_GENERIC_RCU_GUP I don't see it. Thus the question re > what am I missing here... Ok, I definitely agree that it looks like __get_user_pages_fast() just needs to get rid of the access_ok() and replace it with a proper check for the user address space range. Looks like arm[64] and powerpc.are the current users. Adding in some people involved with the original submission a few years ago. I do note that the x86 __get_user_pages_fast() thing looks dodgy too. In particular, we do it right in the *real* get_user_pages_fast(), see commit 7f8189068726 ("x86: don't use 'access_ok()' as a range check in get_user_pages_fast()"). But then the same bug was re-introduced when the "irq safe" version was merged. As well as in the GENERIC_RCU_GUP version. Gaah. Apparently PeterZ copied the old buggy version before the fix when he added __get_user_pages_fast() in commit 465a454f254e ("x86, mm: Add __get_user_pages_fast()"). I guess it could be considered a merge error (both happened during the 2.6.31 merge window). Linus From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call Date: Fri, 3 Feb 2017 11:28:48 -0800 Message-ID: References: <20170124212327.14517-1-jlayton@redhat.com> <20170125133205.21704-1-jlayton@redhat.com> <20170202095125.GF27291@ZenIV.linux.org.uk> <20170202105651.GA32111@infradead.org> <20170202111625.GG27291@ZenIV.linux.org.uk> <1486040452.2812.6.camel@redhat.com> <20170203072952.GI27291@ZenIV.linux.org.uk> <20170203190816.GK27291@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170203190816.GK27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: lustre-devel-bounces-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org Sender: "lustre-devel" To: Al Viro , Steve Capper , Catalin Marinas , Hugh Dickins , Peter Zijlstra , Ingo Molnar Cc: Linux NFS Mailing List , Jan Kara , Jeff Layton , Linux Kernel Mailing List , Chris Wilson , Christoph Hellwig , linux-fsdevel , V9FS Developers , ceph-devel , "Kirill A. Shutemov" , lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org List-Id: ceph-devel.vger.kernel.org On Fri, Feb 3, 2017 at 11:08 AM, Al Viro wrote: > > On x86 it does. I don't see anything equivalent in mm/gup.c one, and the > only kinda-sorta similar thing (access_ok() in __get_user_pages_fast() > there) is vulnerable to e.g. access via kernel_write(). Yeah, access_ok() is bogus. It needs to just check against TASK_SIZE or whatever. > doesn't look promising - access_ok() is never sufficient. Something like > _PAGE_USER tests in x86 one solves that problem, but if anything similar > works for HAVE_GENERIC_RCU_GUP I don't see it. Thus the question re > what am I missing here... Ok, I definitely agree that it looks like __get_user_pages_fast() just needs to get rid of the access_ok() and replace it with a proper check for the user address space range. Looks like arm[64] and powerpc.are the current users. Adding in some people involved with the original submission a few years ago. I do note that the x86 __get_user_pages_fast() thing looks dodgy too. In particular, we do it right in the *real* get_user_pages_fast(), see commit 7f8189068726 ("x86: don't use 'access_ok()' as a range check in get_user_pages_fast()"). But then the same bug was re-introduced when the "irq safe" version was merged. As well as in the GENERIC_RCU_GUP version. Gaah. Apparently PeterZ copied the old buggy version before the fix when he added __get_user_pages_fast() in commit 465a454f254e ("x86, mm: Add __get_user_pages_fast()"). I guess it could be considered a merge error (both happened during the 2.6.31 merge window). Linus From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Date: Fri, 3 Feb 2017 11:28:48 -0800 Subject: [lustre-devel] [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call In-Reply-To: <20170203190816.GK27291@ZenIV.linux.org.uk> References: <20170124212327.14517-1-jlayton@redhat.com> <20170125133205.21704-1-jlayton@redhat.com> <20170202095125.GF27291@ZenIV.linux.org.uk> <20170202105651.GA32111@infradead.org> <20170202111625.GG27291@ZenIV.linux.org.uk> <1486040452.2812.6.camel@redhat.com> <20170203072952.GI27291@ZenIV.linux.org.uk> <20170203190816.GK27291@ZenIV.linux.org.uk> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Al Viro , Steve Capper , Catalin Marinas , Hugh Dickins , Peter Zijlstra , Ingo Molnar Cc: Linux NFS Mailing List , Jan Kara , Jeff Layton , Linux Kernel Mailing List , Chris Wilson , Christoph Hellwig , linux-fsdevel , V9FS Developers , ceph-devel , "Kirill A. Shutemov" , lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org On Fri, Feb 3, 2017 at 11:08 AM, Al Viro wrote: > > On x86 it does. I don't see anything equivalent in mm/gup.c one, and the > only kinda-sorta similar thing (access_ok() in __get_user_pages_fast() > there) is vulnerable to e.g. access via kernel_write(). Yeah, access_ok() is bogus. It needs to just check against TASK_SIZE or whatever. > doesn't look promising - access_ok() is never sufficient. Something like > _PAGE_USER tests in x86 one solves that problem, but if anything similar > works for HAVE_GENERIC_RCU_GUP I don't see it. Thus the question re > what am I missing here... Ok, I definitely agree that it looks like __get_user_pages_fast() just needs to get rid of the access_ok() and replace it with a proper check for the user address space range. Looks like arm[64] and powerpc.are the current users. Adding in some people involved with the original submission a few years ago. I do note that the x86 __get_user_pages_fast() thing looks dodgy too. In particular, we do it right in the *real* get_user_pages_fast(), see commit 7f8189068726 ("x86: don't use 'access_ok()' as a range check in get_user_pages_fast()"). But then the same bug was re-introduced when the "irq safe" version was merged. As well as in the GENERIC_RCU_GUP version. Gaah. Apparently PeterZ copied the old buggy version before the fix when he added __get_user_pages_fast() in commit 465a454f254e ("x86, mm: Add __get_user_pages_fast()"). I guess it could be considered a merge error (both happened during the 2.6.31 merge window). Linus