Re: [RFC][PATCH 2/3] exec: Simplify unshare_files

From: ebiederm@xmission.com (Eric W. Biederman)
To: Oleg Nesterov <oleg@redhat.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 2/3] exec: Simplify unshare_files
Date: Mon, 17 Sep 2018 22:26:15 +0200	[thread overview]
Message-ID: <874lenoopk.fsf@xmission.com> (raw)
In-Reply-To: <20180917162342.GB25565@redhat.com> (Oleg Nesterov's message of "Mon, 17 Sep 2018 18:23:42 +0200")

Oleg Nesterov <oleg@redhat.com> writes:

> absolutely off-topic question,
>
> On 09/16, Eric W. Biederman wrote:
>>
>> @@ -747,11 +746,9 @@ void do_coredump(const siginfo_t *siginfo)
>>  	}
>>
>>  	/* get us an unshared descriptor table; almost always a no-op */
>> -	retval = unshare_files(&displaced);
>> +	retval = unshare_files();
>
> I fail to understand why do_coredump() needs unshare_files(). Could someone
> explain?
>
> And "almost always a no-op" above is not true, this is never a no-op in mt case;
> other (killed) threads sleep in exit_mm() which is called before
> exit_files().

So I looked at the history and I have half an explanation.
179e037fc1370288188cb1f90b81156d75a3cb2d do_coredump(): make sure that descriptor table isn't shared

As far as I can tell this was Al Viro making certain that there were not
any races that had to be dealt with when accessing the file table during
execve.

Which gets to the heart of what we have to do before this set of changes
that we have been looking at can be merged.  We need to go through exec
and do_coredump if we wish to remove this call of unshare_files
and verify that everything is thread-safe, and using thread-safe idioms.

There is at least one place in exec where it is documented that the
access to files is not thread-safe in the comment.  I don't think any of
that is fundamentally hard but that work needs to be done for the rest
of this cleanup to be usable.

Eric