On Nov 19, 2015 12:11 PM, "Kees Cook" <keescook@xxxxxxxxxxxx> wrote: > > On Tue, Nov 10, 2015 at 7:08 AM, Jan Kara <jack@xxxxxxx> 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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
