linux-kernel - Re: [PATCH net-next] modules: allow modprobe load regular elf binaries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 9 Mar 2018 10:50:59 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     David Miller <davem@...emloft.net>,
        Alexei Starovoitov <ast@...com>,
        Andy Lutomirski <luto@...capital.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Djalal Harouni <tixxdz@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Daniel Borkmann <daniel@...earbox.net>,
        Greg KH <gregkh@...uxfoundation.org>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-team <kernel-team@...com>,
        Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH net-next] modules: allow modprobe load regular elf binaries

On Fri, Mar 9, 2018 at 10:43 AM, Kees Cook <keescook@...omium.org> 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