Re: [PATCH] Userfaultfd: Avoid double free of userfault_ctx and remove O_CLOEXEC

Aleksa Sarai
2020-08-06T23:59:50Z

[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]

On 2020-08-04, 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.

Ah, I missed that this was a UAPI breakage. :P

> > At the very least there should be a new flag added that sets O_CLOEXEC.
>
> There already is one (but these patches broke it).

--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]