linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Daniel Colascione <dancol@google.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>,
	Andy Lutomirski <luto@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jann Horn <jannh@google.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Lokesh Gidra <lokeshgidra@google.com>,
	Nick Kralevich <nnk@google.com>, Nosh Minwalla <nosh@google.com>,
	Pavel Emelyanov <ovzxemul@gmail.com>,
	Tim Murray <timmurray@google.com>,
	Linux API <linux-api@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>
Subject: Re: [PATCH 1/1] userfaultfd: require CAP_SYS_PTRACE for UFFD_FEATURE_EVENT_FORK
Date: Thu, 7 Nov 2019 13:22:59 -0500	[thread overview]
Message-ID: <20191107182259.GK17896@redhat.com> (raw)
In-Reply-To: <CAKOZueuKttjyRBgMkaBknzn+fzufZA+gJcd5wnKgiwmO37yN1g@mail.gmail.com>

On Thu, Nov 07, 2019 at 08:15:53AM -0800, Daniel Colascione wrote:
> You're already paying for bounds checking. Receiving a message via a
> datagram socket is basically the same thing as what UFFD's read is
> doing anyway.

Except it's synchronous and there are no dynamic allocations required
in uffd, while af_netlink and af_unix both all deal with queue of
events in skbs dynamically allocated.

Ultimately if we strip away the skbs for performance reasons, there
wouldn't be much code to share, so if the only benefit would be to
call recvmsg which would still be as insecure as read for the "worse"
case than suid, so I don't see the point.

And should then eventfd also become a netlink then? I mean uffd was
supposed to work like eventfd except it requires specialized events.

> Programs generally don't go calling recvmsg() on random FDs they get
> from the outside world. They do call read() on those FDs, which is why

That programs generally don't do something only means the attack is
less probable.

Programs generally aren't suid. Programs generally don't use
SCM_RIGHTS. Programs generally don't ignore the retval of
open/socket/uffd syscalls. Programs generally don't make assumptions
on the fd ID after one of those syscalls that install fds.

If all programs generally do the right thing (where the most important
is to not make assumptions on the fd IDs and to check all syscall
retvals), there was never an issue to begin with even in uffd API.

> read() having unexpected side effects is terrible.

If having unexpected side effects in read is "terrible" (i.e. I
personally prefer to use terms like terrible when there's at least
something that can be exploited in practice, not for theoretical
issues) for an SCM_RIGHTS receiving daemon, I just don't see how the
exact same unexpected (still theoretical) side effects in recvmsg with
an unexpected nested cmsg->cmsg_type == SCM_RIGHTS message being
returned, isn't terrible too.

> If you call it with a non-empty ancillary data buffer, you know to
> react to what you get. You're *opting into* the possibility of getting
> file descriptors. Sure, it's theoretically possible that a program
> calls recvmsg on random FDs it gets from unknown sources, sees
> SCM_RIGHTS unexpectedly, and just the SCM_RIGHTS message and its FD
> payload, but that's an outright bug, while calling read() on stdin is
> no bug.

I'm not talking about stdin and suid. recvmsg might mitigate the
concern for suid (not certain, depends on the suid, if it's generally
doing what you expect most suid to be doing or not), I was talking
about the SCM_RIGHTS receiving daemon instead, the "worse" more
concerning case than the suid.

I quote below Andy's relevant email:

======
It's worse if SCM_RIGHTS is involved.
======

Not all software will do this after calling recvmsg:

    if (cmsg->cmsg_type == SCM_RIGHTS) {
      /* oops we got attacked and an fd was involountarily installed
         because we received another AF_UNIX from a malicious attacker
         in control of the other end of the SCM_RIGHTS-receiving
         AF_UNIX connection instead of our expected socket family
         which doesn't even support SCM_RIGHTS so we would never have
         noticed an fd was installed after recvmsg */
    }



  reply	other threads:[~2019-11-07 18:23 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-05 15:29 [PATCH 0/1] userfaultfd: require CAP_SYS_PTRACE for UFFD_FEATURE_EVENT_FORK Mike Rapoport
2019-11-05 15:29 ` [PATCH 1/1] " Mike Rapoport
2019-11-05 15:37   ` Andrea Arcangeli
2019-11-05 15:55   ` Daniel Colascione
2019-11-05 16:00     ` Andy Lutomirski
2019-11-05 16:06       ` Daniel Colascione
2019-11-05 16:33         ` Andrea Arcangeli
2019-11-05 16:39           ` Daniel Colascione
2019-11-05 16:55             ` Andrea Arcangeli
2019-11-05 17:02               ` Daniel Colascione
2019-11-05 17:30                 ` Andrea Arcangeli
2019-11-05 22:01                 ` Andy Lutomirski
2019-11-05 22:10                   ` Daniel Colascione
2019-11-05 16:24       ` Andrea Arcangeli
2019-11-05 16:41         ` Daniel Colascione
2019-11-07  8:39           ` Mike Rapoport
2019-11-07  8:54             ` Daniel Colascione
2019-11-07 15:38               ` Andrea Arcangeli
2019-11-07 16:15                 ` Daniel Colascione
2019-11-07 18:22                   ` Andrea Arcangeli [this message]
2019-11-07 18:50                     ` Daniel Colascione
2019-11-07 19:27                       ` Andrea Arcangeli
2019-11-10 17:02                     ` Andy Lutomirski
2019-11-05 15:59   ` Aleksa Sarai

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=20191107182259.GK17896@redhat.com \
    --to=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dancol@google.com \
    --cc=jannh@google.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lokeshgidra@google.com \
    --cc=luto@kernel.org \
    --cc=nnk@google.com \
    --cc=nosh@google.com \
    --cc=ovzxemul@gmail.com \
    --cc=rppt@linux.ibm.com \
    --cc=timmurray@google.com \
    --cc=torvalds@linux-foundation.org \
    /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).