From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Tue, 9 May 2017 23:53:01 -0700 From: Christoph Hellwig Message-ID: <20170510065301.GC4115@infradead.org> References: <20170508073352.caqe3fqf7nuxypgi@gmail.com> <20170508124621.GA20705@kroah.com> <20170509064522.anusoikaalvlux3w@gmail.com> <20170509085659.GA32555@infradead.org> <20170509130250.GA11381@infradead.org> <20170509160322.GA15902@infradead.org> <20170510021118.GA390@ZenIV.linux.org.uk> <20170510024524.GB390@ZenIV.linux.org.uk> <20170510031254.GC390@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170510031254.GC390@ZenIV.linux.org.uk> Subject: Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode To: Al Viro Cc: Christoph Hellwig , Andy Lutomirski , Ingo Molnar , Greg KH , Thomas Garnier , Martin Schwidefsky , Heiko Carstens , Dave Hansen , Arnd Bergmann , Thomas Gleixner , David Howells , =?iso-8859-1?Q?Ren=E9?= Nyffenegger , Andrew Morton , "Paul E . McKenney" , "Eric W . Biederman" , Oleg Nesterov , Pavel Tikhomirov , Ingo Molnar , "H . Peter Anvin" , Paolo Bonzini , Rik van Riel , Kees Cook , Josh Poimboeuf , Borislav Petkov , Brian Gerst , "Kirill A . Shutemov" , Christian Borntraeger , Russell King , Will Deacon , Catalin Marinas , Mark Rutland , James Morse , linux-s390 , LKML , Linux API , the arch/x86 maintainers , "linux-arm-kernel@lists.infradead.org" , Kernel Hardening , Linus Torvalds , Peter Zijlstra List-ID: On Wed, May 10, 2017 at 04:12:54AM +0100, Al Viro wrote: > What's the point? What's wrong with having kernel_read()/kernel_readv()/etc.? > You still have set_fs() in there; doing that one level up in call chain would > be just fine... IDGI. The problem is that they modify the address limit, which the whole subthread here wants to get rid of. > Broken commit: "net: don't play with address limits in kernel_recvmsg". > It would be OK if it was only about data. Unfortunately, that's not > true in one case: svc_udp_recvfrom() wants ->msg_control. Dropped, but we'll need to fix that eventually. > Another delicate place: you can't assume that write() always advances > file position by its (positive) return value. btrfs stuff is sensitive > to that. If we don't want to assume that we need to pass pointer to pos to kernel_read/write. Which might be a good idea in general. > ashmem probably _is_ OK with demanding ->read_iter(), but I'm not sure > about blind asma->file->f_pos += ret. That's begging for races. Actually, > scratch that - it *is* racy. I think the proper fix is to not even bother to maintain f_pos of the backing file, as we don't ever use it - all reads from it pass in an explicit position anyway. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode Date: Tue, 9 May 2017 23:53:01 -0700 Message-ID: <20170510065301.GC4115@infradead.org> References: <20170508073352.caqe3fqf7nuxypgi@gmail.com> <20170508124621.GA20705@kroah.com> <20170509064522.anusoikaalvlux3w@gmail.com> <20170509085659.GA32555@infradead.org> <20170509130250.GA11381@infradead.org> <20170509160322.GA15902@infradead.org> <20170510021118.GA390@ZenIV.linux.org.uk> <20170510024524.GB390@ZenIV.linux.org.uk> <20170510031254.GC390@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20170510031254.GC390@ZenIV.linux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Al Viro Cc: Mark Rutland , Kernel Hardening , Greg KH , Heiko Carstens , LKML , David Howells , Dave Hansen , "H . Peter Anvin" , Ingo Molnar , Pavel Tikhomirov , Peter Zijlstra , linux-s390 , the arch/x86 maintainers , Russell King , Will Deacon , Christoph Hellwig , Christian Borntraeger , =?iso-8859-1?Q?Ren=E9?= Nyffenegger , Catalin Marinas , "Paul E . McKenney" , Rik van Riel , Kees Cook List-Id: linux-api@vger.kernel.org On Wed, May 10, 2017 at 04:12:54AM +0100, Al Viro wrote: > What's the point? What's wrong with having kernel_read()/kernel_readv()/= etc.? > You still have set_fs() in there; doing that one level up in call chain w= ould > be just fine... IDGI. The problem is that they modify the address limit, which the whole subthread here wants to get rid of. > Broken commit: "net: don't play with address limits in kernel_recvmsg". > It would be OK if it was only about data. Unfortunately, that's not > true in one case: svc_udp_recvfrom() wants ->msg_control. Dropped, but we'll need to fix that eventually. > Another delicate place: you can't assume that write() always advances > file position by its (positive) return value. btrfs stuff is sensitive > to that. If we don't want to assume that we need to pass pointer to pos to kernel_read/write. Which might be a good idea in general. > ashmem probably _is_ OK with demanding ->read_iter(), but I'm not sure > about blind asma->file->f_pos +=3D ret. That's=A0begging for races. Act= ually, > scratch that - it *is* racy. I think the proper fix is to not even bother to maintain f_pos of the backing file, as we don't ever use it - all reads from it pass in an explicit position anyway. From mboxrd@z Thu Jan 1 00:00:00 1970 From: hch@infradead.org (Christoph Hellwig) Date: Tue, 9 May 2017 23:53:01 -0700 Subject: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode In-Reply-To: <20170510031254.GC390@ZenIV.linux.org.uk> References: <20170508073352.caqe3fqf7nuxypgi@gmail.com> <20170508124621.GA20705@kroah.com> <20170509064522.anusoikaalvlux3w@gmail.com> <20170509085659.GA32555@infradead.org> <20170509130250.GA11381@infradead.org> <20170509160322.GA15902@infradead.org> <20170510021118.GA390@ZenIV.linux.org.uk> <20170510024524.GB390@ZenIV.linux.org.uk> <20170510031254.GC390@ZenIV.linux.org.uk> Message-ID: <20170510065301.GC4115@infradead.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, May 10, 2017 at 04:12:54AM +0100, Al Viro wrote: > What's the point? What's wrong with having kernel_read()/kernel_readv()/etc.? > You still have set_fs() in there; doing that one level up in call chain would > be just fine... IDGI. The problem is that they modify the address limit, which the whole subthread here wants to get rid of. > Broken commit: "net: don't play with address limits in kernel_recvmsg". > It would be OK if it was only about data. Unfortunately, that's not > true in one case: svc_udp_recvfrom() wants ->msg_control. Dropped, but we'll need to fix that eventually. > Another delicate place: you can't assume that write() always advances > file position by its (positive) return value. btrfs stuff is sensitive > to that. If we don't want to assume that we need to pass pointer to pos to kernel_read/write. Which might be a good idea in general. > ashmem probably _is_ OK with demanding ->read_iter(), but I'm not sure > about blind asma->file->f_pos += ret. That's?begging for races. Actually, > scratch that - it *is* racy. I think the proper fix is to not even bother to maintain f_pos of the backing file, as we don't ever use it - all reads from it pass in an explicit position anyway.