From: Oleg Nesterov <oleg@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: viro@zeniv.linux.org.uk, ebiederm@xmission.com,
berrange@redhat.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v3 3/3] exec: do unshare_files after de_thread
Date: Sat, 15 Sep 2018 18:37:04 +0200 [thread overview]
Message-ID: <20180915163704.GA31693@redhat.com> (raw)
In-Reply-To: <20180914105310.6454-4-jlayton@kernel.org>
On 09/14, Jeff Layton wrote:
>
> POSIX mandates that open fds and their associated file locks should be
> preserved across an execve. This works, unless the process is
> multithreaded at the time that execve is called.
>
> In that case, we'll end up unsharing the files_struct but the locks will
> still have their fl_owner set to the address of the old one. Eventually,
> when the other threads die and the last reference to the old
> files_struct is put, any POSIX locks get torn down since it looks like
> a close occurred on them.
>
> The result is that all of your open files will be intact with none of
> the locks you held before execve. The simple answer to this is "use OFD
> locks", but this is a nasty surprise and it violates the spec.
>
> Fix this by doing unshare_files later during exec,
See my reply to 1/3... if we can forget about the races with get_files_struct()
we can probably make a much simpler patch, plus we do not need 2/2, afaics.
What I really can't understand is why we need to _change_ current->files
early in do_execve().
IOW. Lets ignore do_close_on_exec(), lets ignore the fact that unshare_fd()
can fail and thus it makes sense to call it before point-of-no-return.
Any other reason why we can't simply call unshare_files() at the end of
__do_execve_file() on success?
Oleg.
next prev parent reply other threads:[~2018-09-15 21:56 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-14 10:53 [PATCH v3 0/3] exec: fix passing of file locks across execve in multithreaded processes Jeff Layton
2018-09-14 10:53 ` [PATCH v3 1/3] exec: separate thread_count for files_struct Jeff Layton
2018-09-15 16:04 ` Oleg Nesterov
2018-09-16 16:10 ` Eric W. Biederman
2018-09-17 15:24 ` Oleg Nesterov
2018-09-17 20:45 ` Eric W. Biederman
2018-09-14 10:53 ` [PATCH v3 2/3] exec: delay clone(CLONE_FILES) if task associated with current files_struct is exec'ing Jeff Layton
2018-09-14 10:53 ` [PATCH v3 3/3] exec: do unshare_files after de_thread Jeff Layton
2018-09-15 16:37 ` Oleg Nesterov [this message]
2018-09-16 16:49 ` Eric W. Biederman
2018-09-17 15:28 ` Oleg Nesterov
2018-09-16 16:59 ` Eric W. Biederman
2018-09-16 17:38 ` [RFC][PATCH 0/3] exec: Moving unshare_files_struct Eric W. Biederman
2018-09-16 17:39 ` [RFC][PATCH 1/3] exec: Move unshare_files down to avoid locks being dropped on exec Eric W. Biederman
2018-09-17 15:49 ` Oleg Nesterov
2018-09-16 17:40 ` [RFC][PATCH 2/3] exec: Simplify unshare_files Eric W. Biederman
2018-09-17 16:23 ` Oleg Nesterov
2018-09-17 20:26 ` Eric W. Biederman
2018-09-16 17:41 ` [RFC][PATCH 3/3] exec: Remove reset_files_struct Eric W. Biederman
2018-09-17 15:59 ` [RFC][PATCH 0/3] exec: Moving unshare_files_struct Oleg Nesterov
2018-09-18 22:18 ` Eric W. Biederman
2018-09-17 16:24 ` Jeff Layton
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=20180915163704.GA31693@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=berrange@redhat.com \
--cc=ebiederm@xmission.com \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.