linux-kernel - Re: [RFC][PATCH 1/3] exec: Move unshare_files down to avoid locks being dropped on exec.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180917154938.GC25173@redhat.com>
Date:   Mon, 17 Sep 2018 17:49:38 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Jeff Layton <jlayton@...nel.org>, viro@...iv.linux.org.uk,
        berrange@...hat.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH 1/3] exec: Move unshare_files down to avoid locks
 being dropped on exec.

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.