[CRIU] [PATCH v2 02/24] exec: Simplify unshare_files

[Eric W. Biederman]

Oleg Nesterov <oleg at redhat.com> writes:

> I'll try to actually read this series tomorrow but I see nothing wrong
> after the quick glance.
>
> Just one off-topic question,
>
> On 11/20, Eric W. Biederman wrote:
>>
>> --- a/fs/coredump.c
>> +++ b/fs/coredump.c
>> @@ -585,7 +585,6 @@ void do_coredump(const kernel_siginfo_t *siginfo)
>>  	int ispipe;
>>  	size_t *argv = NULL;
>>  	int argc = 0;
>> -	struct files_struct *displaced;
>>  	/* require nonrelative corefile path and be extra careful */
>>  	bool need_suid_safe = false;
>>  	bool core_dumped = false;
>> @@ -791,11 +790,9 @@ void do_coredump(const kernel_siginfo_t *siginfo)
>>  	}
>>
>>  	/* get us an unshared descriptor table; almost always a no-op */
>> -	retval = unshare_files(&displaced);
>> +	retval = unshare_files();
>
> Can anyone explain why does do_coredump() need unshare_files() at all?

A very good question.  I see the commit where this change came in
179e037fc137 ("do_coredump(): make sure that descriptor table isn't
shared").  But that doesn't give much information.

I see the obvious that unshare_files keeps us from racing with process
that share files that are not part of the core dump.  However in
fs/coredump.c and fs/binfmt_elf.c I don't see anywhere that we actually
dump the set of files the program has open so I don't know what the
point of preventing races is.

So I we can verify that none of the other core dump handlers cares and
remove unshare_files entirely from do_coredump.

Eric