From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [PATCH net-next] modules: allow modprobe load regular elf binaries Date: Fri, 9 Mar 2018 10:50:59 -0800 Message-ID: References: <87478c51-59a7-f6ac-1fb2-f3ca2dcf658b@fb.com> <20180309.133509.1275903267249306409.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Kees Cook Cc: David Miller , Alexei Starovoitov , Andy Lutomirski , Alexei Starovoitov , Djalal Harouni , Al Viro , Daniel Borkmann , Greg KH , "Luis R. Rodriguez" , Network Development , LKML , kernel-team , Linux API List-Id: linux-api@vger.kernel.org On Fri, Mar 9, 2018 at 10:43 AM, Kees Cook wrote: > > Module loading (via kernel_read_file()) already uses > deny_write_access(), and so does do_open_execat(). As long as module > loading doesn't call allow_write_access() before the execve() has > started in the new implementation, I think we'd be covered here. No. kernel_read_file() only does it *during* the read. So there's a huge big honking gap between the two. Also, the second part of my suggestion was to be entirely synchronous with the whole execution of the process, and do it within the "we do mutual exclusion fo rmodules with the same name" logic. Note that Andrei's patch uses UMH_WAIT_EXEC. That's basically "vfork+exec" - it only waits for the exec to have started, it doesn't wait for the whole thing. So I'm saying "use UMH_WAIT_PROC, do it in a different place, and make sure you cover the whole sequence with deny_write_access()". Linus