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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87478c51-59a7-f6ac-1fb2-f3ca2dcf658b@fb.com>
Date:   Thu, 8 Mar 2018 21:08:17 -0800
From:   Alexei Starovoitov <ast@...com>
To:     Andy Lutomirski <luto@...capital.net>,
        Linus Torvalds <torvalds@...ux-foundation.org>
CC:     Kees Cook <keescook@...omium.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Djalal Harouni <tixxdz@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        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 3/8/18 7:54 PM, Andy Lutomirski wrote:
>
>
>
>> On Mar 8, 2018, at 7:06 PM, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
>>
>>
>> Honestly, that "read twice" thing may be what scuttles this.
>> Initially, I thought it was a non-issue, because anybody who controls
>> the module subdirectory enough to rewrite files would be in a position
>> to just execute the file itself directly instead.
>>
>
> On further consideration, I think there’s another showstopper. This patch is a potentially severe ABI break. Right now, loading a module *copies* it into memory and does not hold a reference to the underlying fs. With the patch applied, all kinds of use cases can break in gnarly ways. Initramfs is maybe okay, but initrd may be screwed. If you load an ET_EXEC module from initrd, then umount it, then clear the ramdisk, something will go horribly wrong. Exactly what goes wrong depends on whether userspace notices that umount() failed. Similarly, if you load one of these modules over a network and then lose your connection, you have a problem.

there is not abi breakage and file cannot disappear from running task.
One cannot umount fs while file is still being used.

>
> The “read twice” thing is also bad for another reason: containers. Suppose I have a setup where a container can load a signed module blob. With the read twice code, the container can race and run an entirely different blob outside the container.

Not only "read twice", but "read many".
If .text sections of elf that are not yet in memory can be modified
by malicious user, later they will be brought in with different code.
I think the easiest fix to tighten this "umh modules" to CAP_SYS_ADMIN.