From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:48122 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728313AbeINQHM (ORCPT ); Fri, 14 Sep 2018 12:07:12 -0400 From: Jeff Layton To: viro@zeniv.linux.org.uk Cc: ebiederm@xmission.com, berrange@redhat.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3 2/3] exec: delay clone(CLONE_FILES) if task associated with current files_struct is exec'ing Date: Fri, 14 Sep 2018 06:53:09 -0400 Message-Id: <20180914105310.6454-3-jlayton@kernel.org> In-Reply-To: <20180914105310.6454-1-jlayton@kernel.org> References: <20180914105310.6454-1-jlayton@kernel.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: In a later patch, we're going to move the unshare_files call in __do_execve_file to later in the process, but this opens up a potential race with clone(CLONE_FILES). We could end up bumping the refcount on the files_struct after we've checked to see if it was shared. What we really want to do in that case is have the clone return -EAGAIN, much like the handling of the similar situation in copy_fs. Add a new in_exec boolean to files_struct that is protected by the file_lock. Set it if the files_struct turns out not to be shared, and clear at the end of __do_execve_file. If it's set when we go to copy_process, then just return -EAGAIN. Cc: Eric W. Biederman Signed-off-by: Jeff Layton --- fs/exec.c | 14 ++++++++++++-- fs/file.c | 1 + include/linux/fdtable.h | 1 + kernel/fork.c | 21 ++++++++++++++------- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index e71a70d70158..c8a68481d7eb 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1831,8 +1831,13 @@ static int __do_execve_file(int fd, struct filename *filename, kfree(pathbuf); if (filename) putname(filename); - if (displaced) + if (displaced) { release_files_struct(displaced); + } else { + spin_lock(¤t->files->file_lock); + current->files->in_exec = false; + spin_unlock(¤t->files->file_lock); + } return retval; out: @@ -1850,8 +1855,13 @@ static int __do_execve_file(int fd, struct filename *filename, kfree(pathbuf); out_files: - if (displaced) + if (displaced) { reset_files_struct(displaced); + } else { + spin_lock(¤t->files->file_lock); + current->files->in_exec = false; + spin_unlock(¤t->files->file_lock); + } out_ret: if (filename) putname(filename); diff --git a/fs/file.c b/fs/file.c index e35eeff4593c..535321846425 100644 --- a/fs/file.c +++ b/fs/file.c @@ -286,6 +286,7 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp) spin_lock_init(&newf->file_lock); newf->thread_count = 1; newf->resize_in_progress = false; + newf->in_exec = false; init_waitqueue_head(&newf->resize_wait); newf->next_fd = 0; new_fdt = &newf->fdtab; diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index 2cb02f438201..a09ba1b20693 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -51,6 +51,7 @@ struct files_struct { */ atomic_t count; bool resize_in_progress; + bool in_exec; wait_queue_head_t resize_wait; struct fdtable __rcu *fdt; diff --git a/kernel/fork.c b/kernel/fork.c index c7eb63e4d5c1..a3786efb39f1 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1372,10 +1372,15 @@ static int copy_files(unsigned long clone_flags, struct task_struct *tsk) goto out; if (clone_flags & CLONE_FILES) { - atomic_inc(&oldf->count); spin_lock(&oldf->file_lock); - oldf->thread_count++; - spin_unlock(&oldf->file_lock); + if (oldf->in_exec) { + spin_unlock(&oldf->file_lock); + error = -EAGAIN; + } else { + oldf->thread_count++; + spin_unlock(&oldf->file_lock); + atomic_inc(&oldf->count); + } goto out; } @@ -2419,18 +2424,20 @@ static int unshare_fs(unsigned long unshare_flags, struct fs_struct **new_fsp) static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp) { struct files_struct *fd = current->files; - int error, count; + int error; if (!(unshare_flags & CLONE_FILES) || !fd) return 0; spin_lock(&fd->file_lock); - count = fd->thread_count; - spin_unlock(&fd->file_lock); - if (count > 1) { + if (fd->thread_count > 1) { + spin_unlock(&fd->file_lock); *new_fdp = dup_fd(fd, &error); if (!*new_fdp) return error; + } else { + fd->in_exec = true; + spin_unlock(&fd->file_lock); } return 0; -- 2.17.1