From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Date: Mon, 17 Sep 2018 17:49:38 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Jeff Layton <jlayton@kernel.org>, viro@zeniv.linux.org.uk,
        berrange@redhat.com, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH 1/3] exec: Move unshare_files down to avoid locks
 being dropped on exec.
Message-ID: <20180917154938.GC25173@redhat.com>
References: <20180914105310.6454-1-jlayton@kernel.org>
 <87a7ohs5ow.fsf@xmission.com>
 <874leps5n9.fsf_-_@xmission.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <874leps5n9.fsf_-_@xmission.com>
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On 09/16, Eric W. Biederman wrote:
>
> @@ -1291,6 +1292,12 @@ int flush_old_exec(struct linux_binprm * bprm)
>  	flush_thread();
>  	current->personality &= ~bprm->per_clear;
>
> +	retval = unshare_files(&displaced);

I was going to sugget basically the same changes, please feel free to add
my reviewed-by to 1-3.


Just for record. If we should really worry about unshare_files() failure
after de_thread() (imo we shouldn't), we can do another change:

	__do_execve_file:

		unshare_fd(CLONE_FILES, &bprm->unshared_copy);
		...


	flush_old_exec:

		de_thread();

		if (bprm->unshared_copy) {
			// now that we killed sub-threads recheck
			if (current->files->count > 1) {
				put_files_struct(current->files);
				current->files = bprm->unshared_copy;
			} else {
				put_files_struct(bprm->unshared_copy);
			}
				
		}

but again, I think your series is fine.

Oleg.