From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Wed, 10 May 2017 04:39:12 +0100 From: Al Viro Message-ID: <20170510033911.GE390@ZenIV.linux.org.uk> References: <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> <20170510032137.GD390@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: <20170510032137.GD390@ZenIV.linux.org.uk> Sender: Al Viro Subject: Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode To: Christoph Hellwig Cc: 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:21:37AM +0100, Al Viro wrote: > On Wed, May 10, 2017 at 04:12:54AM +0100, Al Viro wrote: > > > 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. > > > > Another delicate place: you can't assume that write() always advances > > file position by its (positive) return value. btrfs stuff is sensitive > > to that. > > > > 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. > > kvec_length(): please, don't. I would rather have the last remaining > iov_length() gone... What do you need it for, anyway? You have only > two users and both have the count passed to them (as *count and *cnt resp.) fcntl stuff: I've decided not to put something similar into work.compat since I couldn't decide what to do with compat stuff - word-by-word copy from userland converting to struct flock + conversion to posix_lock + actual work + conversion to flock + word-by-word copy to userland... Smells like we might be better off with compat_flock_to_posix_lock() et.al. I'm still not sure; played a bit one way and another and dediced to drop it for now. Hell knows... From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode Date: Wed, 10 May 2017 04:39:12 +0100 Message-ID: <20170510033911.GE390@ZenIV.linux.org.uk> References: <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> <20170510032137.GD390@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Return-path: Content-Disposition: inline In-Reply-To: <20170510032137.GD390-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org> Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Christoph Hellwig Cc: 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 List-Id: linux-api@vger.kernel.org On Wed, May 10, 2017 at 04:21:37AM +0100, Al Viro wrote: > On Wed, May 10, 2017 at 04:12:54AM +0100, Al Viro wrote: > > > 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. > > > > Another delicate place: you can't assume that write() always advances > > file position by its (positive) return value. btrfs stuff is sensitive > > to that. > > > > 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. > > kvec_length(): please, don't. I would rather have the last remaining > iov_length() gone... What do you need it for, anyway? You have only > two users and both have the count passed to them (as *count and *cnt resp.) fcntl stuff: I've decided not to put something similar into work.compat since I couldn't decide what to do with compat stuff - word-by-word copy from userland converting to struct flock + conversion to posix_lock + actual work + conversion to flock + word-by-word copy to userland... Smells like we might be better off with compat_flock_to_posix_lock() et.al. I'm still not sure; played a bit one way and another and dediced to drop it for now. Hell knows... From mboxrd@z Thu Jan 1 00:00:00 1970 From: viro@ZenIV.linux.org.uk (Al Viro) Date: Wed, 10 May 2017 04:39:12 +0100 Subject: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode In-Reply-To: <20170510032137.GD390@ZenIV.linux.org.uk> References: <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> <20170510032137.GD390@ZenIV.linux.org.uk> Message-ID: <20170510033911.GE390@ZenIV.linux.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, May 10, 2017 at 04:21:37AM +0100, Al Viro wrote: > On Wed, May 10, 2017 at 04:12:54AM +0100, Al Viro wrote: > > > 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. > > > > Another delicate place: you can't assume that write() always advances > > file position by its (positive) return value. btrfs stuff is sensitive > > to that. > > > > 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. > > kvec_length(): please, don't. I would rather have the last remaining > iov_length() gone... What do you need it for, anyway? You have only > two users and both have the count passed to them (as *count and *cnt resp.) fcntl stuff: I've decided not to put something similar into work.compat since I couldn't decide what to do with compat stuff - word-by-word copy from userland converting to struct flock + conversion to posix_lock + actual work + conversion to flock + word-by-word copy to userland... Smells like we might be better off with compat_flock_to_posix_lock() et.al. I'm still not sure; played a bit one way and another and dediced to drop it for now. Hell knows...