linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@chromium.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Djalal Harouni <tixxdz@gmail.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Greg KH <gregkh@linuxfoundation.org>,
	"Luis R. Rodriguez" <mcgrof@kernel.org>,
	Network Development <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-team <kernel-team@fb.com>,
	Linux API <linux-api@vger.kernel.org>
Subject: Re: [PATCH net-next] modules: allow modprobe load regular elf binaries
Date: Thu, 8 Mar 2018 19:54:20 -0800	[thread overview]
Message-ID: <C94A45E9-54A0-446D-86D6-82D06E2A35CD@amacapital.net> (raw)
In-Reply-To: <CA+55aFwRGDPvaCUgMtqnQHb2PZ77JSMQMQVf1znSN+PiV7NR-Q@mail.gmail.com>




> On Mar 8, 2018, at 7:06 PM, Linus Torvalds <torvalds@linux-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. 


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. 

  parent reply	other threads:[~2018-03-09  3:54 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-06  1:34 [PATCH net-next] modules: allow modprobe load regular elf binaries Alexei Starovoitov
2018-03-06  2:13 ` Randy Dunlap
2018-03-06  3:02   ` Alexei Starovoitov
2018-03-06 11:05 ` Greg KH
2018-03-07  1:07   ` Alexei Starovoitov
2018-03-07  3:24     ` Greg KH
2018-03-06 19:12 ` Linus Torvalds
2018-03-06 23:42   ` Chris Mason
2018-05-02  9:12     ` Jesper Dangaard Brouer
2018-03-06 20:01 ` Andy Lutomirski
2018-03-06 20:26   ` Linus Torvalds
2018-03-07 17:22 ` David Miller
2018-03-08  1:23 ` Luis R. Rodriguez
2018-03-08 23:07   ` Alexei Starovoitov
2018-03-09  1:58     ` Luis R. Rodriguez
2018-03-09  0:24 ` Kees Cook
2018-03-09  0:57   ` Alexei Starovoitov
2018-03-09  1:04     ` Andy Lutomirski
2018-03-09  1:25       ` Alexei Starovoitov
2018-03-09  1:24     ` Kees Cook
2018-03-09  0:59   ` Andy Lutomirski
2018-03-09  1:20     ` Alexei Starovoitov
2018-03-09  2:12       ` Andy Lutomirski
2018-03-09  2:31         ` David Miller
2018-03-09  3:10           ` Andy Lutomirski
2018-03-09  3:27         ` Alexei Starovoitov
2018-03-09  1:38     ` Linus Torvalds
2018-03-09  1:44       ` Kees Cook
2018-03-09  3:06         ` Linus Torvalds
2018-03-09  3:17           ` Linus Torvalds
2018-03-09  3:54           ` Andy Lutomirski [this message]
2018-03-09  5:08             ` Alexei Starovoitov
2018-03-09 15:16               ` Andy Lutomirski
2018-03-09 15:39                 ` Alexei Starovoitov
2018-03-09 16:24                   ` Andy Lutomirski
2018-03-09 17:32                     ` Alexei Starovoitov
2018-03-09 18:15                       ` Greg KH
2018-03-09 18:23                         ` Andy Lutomirski
2018-03-09 18:29                           ` Greg KH
2018-03-09 18:50                           ` Alexei Starovoitov
2018-03-09 18:55                             ` David Miller
2018-03-09 19:37                               ` Andy Lutomirski
2018-03-10  1:43                                 ` Alexei Starovoitov
2018-03-11  2:17                                   ` Andy Lutomirski
2018-03-09 18:17               ` Linus Torvalds
2018-03-09 18:35                 ` David Miller
2018-03-09 18:43                   ` Kees Cook
2018-03-09 18:50                     ` Linus Torvalds
2018-03-09 18:54                       ` Kees Cook
2018-03-09 18:58                       ` Alexei Starovoitov
2018-03-12 12:02                         ` Edward Cree
2018-03-12 17:49                           ` Alexei Starovoitov
2018-03-09 18:48                 ` Andy Lutomirski
2018-03-09 18:53                   ` Linus Torvalds
2018-03-09 18:57                     ` David Miller
2018-03-09 19:12                       ` Linus Torvalds
2018-03-09 19:38                         ` Linus Torvalds
2018-03-09 19:45                           ` Andy Lutomirski
2018-03-10  2:34                           ` Alexei Starovoitov
2018-03-10 14:08                             ` Luis R. Rodriguez
2018-03-10 15:16                               ` Luis R. Rodriguez
2018-03-10 15:34                                 ` Luis R. Rodriguez
2018-03-12 17:22                                   ` Alexei Starovoitov
2018-03-13  8:48                                     ` Greg Kroah-Hartman
2018-03-22 20:54                                 ` Luis R. Rodriguez
2018-03-22 22:15                                   ` Andy Lutomirski
2018-03-22 22:21                                     ` Alexei Starovoitov
2018-03-23  2:47                                     ` Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C94A45E9-54A0-446D-86D6-82D06E2A35CD@amacapital.net \
    --to=luto@amacapital.net \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@fb.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tixxdz@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).