Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759368AbbKSV5d (ORCPT ); Thu, 19 Nov 2015 16:57:33 -0500 Received: from mail-oi0-f49.google.com ([209.85.218.49]:36180 "EHLO mail-oi0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756004AbbKSV53 (ORCPT ); Thu, 19 Nov 2015 16:57:29 -0500 MIME-Version: 1.0 In-Reply-To: References: <20151104065820.GF21740@1wt.eu> <20151108020206.GA3880@thunk.org> <20151110150829.GC3156@quack.suse.cz> From: Andy Lutomirski Date: Thu, 19 Nov 2015 13:57:09 -0800 Message-ID: Subject: Re: [RFC] namei: prevent sgid-hardlinks for unmapped gids To: Kees Cook Cc: "security@kernel.org" , Serge Hallyn , "Eric W . Biederman" , "Theodore Ts'o" , Jan Kara , Theodore Tso , Dirk Steinmetz , Serge Hallyn , Willy Tarreau , Alexander Viro , Michael Kerrisk-manpages , Linux FS Devel , LKML , Seth Forshee Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1890 Lines: 42 On Nov 19, 2015 12:11 PM, "Kees Cook" wrote: > > On Tue, Nov 10, 2015 at 7:08 AM, Jan Kara wrote: > > On Sat 07-11-15 21:02:06, Ted Tso wrote: > >> On Fri, Nov 06, 2015 at 09:05:57PM -0800, Kees Cook wrote: > >> > >>>> They're certainly not used early enough -- we need to remove suid when > >> > >>>> the page becomes writable via mmap (wp_page_shared), not when > >> > >>>> writeback happens, or at least not only when writeback happens. > >> > >>> > >> > >>> Well, I'm shy about the change there. For example, we don't strip in > >> > >>> on open(RDWR), just on write(). > >> > >> > >> > >> I take it back. Hooking wp_page_shared looks expensive. :) Maybe we do > >> > >> need to hook the mmap? > >> > > > >> > > But file_update_time already pokes at the same (or nearby) cachelines, > >> > > I think -- why would it be expensive? The whole thing could be > >> > > guarded by if (unlikely(is setuid)), right? > >> > > >> > Yeah, true. I added file_remove_privs calls near all the > >> > file_update_time calls, to no effect. Added to wp_page_shared too, > >> > nothing. Hmmm. > >> > >> Why not put the the should_remove_suid() call in > >> filemap_page_mkwrite(), or maybe do_page_mkwrite()? > > > > page_mkwrite() callbacks are IMHO the right place for this check (and > > change). Just next to file_update_time() call. You get proper filesystem > > Should file_update_time() just be modified to include > file_remove_privs()? They seem to regularly go together. > No, I think. The current file_update_time is slow and POSIX-noncompliant, and I have old patches I need to dig up to fix it. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/