Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lokesh Gidra <lokeshgidra@google.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Stephen Smalley <stephen.smalley.work@gmail.com>,
	casey@schaufler-ca.com, James Morris <jmorris@namei.org>,
	Kalesh Singh <kaleshsingh@google.com>,
	Daniel Colascione <dancol@dancol.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Nick Kralevich <nnk@google.com>,
	Jeffrey Vander Stoep <jeffv@google.com>,
	Calin Juravle <calin@google.com>,
	kernel-team@android.com, yanfei.xu@windriver.com,
	syzbot+75867c44841cb6373570@syzkaller.appspotmail.com
Subject: Re: [PATCH] Userfaultfd: Avoid double free of userfault_ctx and remove O_CLOEXEC
Date: Tue, 4 Aug 2020 21:54:03 -0700	[thread overview]
Message-ID: <CA+EESO6hds3EJY-MWKiG3tRJfJnyTr4Y_v9+hu5zU1=jiQ_xmQ@mail.gmail.com> (raw)
In-Reply-To: <20200805040806.GB1136@sol.localdomain>

On Tue, Aug 4, 2020 at 9:08 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Aug 05, 2020 at 01:47:58PM +1000, Aleksa Sarai wrote:
> > On 2020-08-04, Lokesh Gidra <lokeshgidra@google.com> wrote:
> > > when get_unused_fd_flags returns error, ctx will be freed by
> > > userfaultfd's release function, which is indirectly called by fput().
> > > Also, if anon_inode_getfile_secure() returns an error, then
> > > userfaultfd_ctx_put() is called, which calls mmdrop() and frees ctx.
> > >
> > > Also, the O_CLOEXEC was inadvertently added to the call to
> > > get_unused_fd_flags() [1].
> >
> > I disagree that it is "wrong" to do O_CLOEXEC-by-default (after all,
> > it's trivial to disable O_CLOEXEC, but it's non-trivial to enable it on
> > an existing file descriptor because it's possible for another thread to
> > exec() before you set the flag). Several new syscalls and fd-returning
> > facilities are O_CLOEXEC-by-default now (the most obvious being pidfds
> > and seccomp notifier fds).
>
> Sure, O_CLOEXEC *should* be the default, but this is an existing syscall so it
> has to keep the existing behavior.
>
> > At the very least there should be a new flag added that sets O_CLOEXEC.
>
> There already is one (but these patches broke it).
>
I looked at the existing implementation, and the right thing is to
pass on the 'flags' (that is passed in to the syscall) to fetch 'fd'.

Besides, as you said in the other email thread,
anon_inode_getfile_secure() should be replaced with
anon_inode_getfd_secure(), which will remove this ambiguity.

I'll resend the patch series soon with all the changes that you proposed.
> - Eric

  reply	other threads:[~2020-08-05  4:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-04 20:31 Lokesh Gidra
2020-08-04 20:45 ` Eric Biggers
2020-08-04 20:49   ` Lokesh Gidra
2020-08-04 20:58     ` Eric Biggers
2020-08-04 23:34       ` Lokesh Gidra
2020-08-05  3:47 ` Aleksa Sarai
2020-08-05  4:08   ` Eric Biggers
2020-08-05  4:54     ` Lokesh Gidra [this message]
2020-08-06 23: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='CA+EESO6hds3EJY-MWKiG3tRJfJnyTr4Y_v9+hu5zU1=jiQ_xmQ@mail.gmail.com' \
    --to=lokeshgidra@google.com \
    --cc=calin@google.com \
    --cc=casey@schaufler-ca.com \
    --cc=cyphar@cyphar.com \
    --cc=dancol@dancol.org \
    --cc=ebiggers@kernel.org \
    --cc=jeffv@google.com \
    --cc=jmorris@namei.org \
    --cc=kaleshsingh@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nnk@google.com \
    --cc=stephen.smalley.work@gmail.com \
    --cc=surenb@google.com \
    --cc=syzbot+75867c44841cb6373570@syzkaller.appspotmail.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yanfei.xu@windriver.com \
    --subject='Re: [PATCH] Userfaultfd: Avoid double free of userfault_ctx and remove O_CLOEXEC' \
    /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

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).