Re: [PATCH] fcntl: make F_GETOWN(EX) return 0 on dead owner task

Pavel Tikhomirov
2021-02-03T21:36:38Z

On 2/3/21 10:32 PM, Cyrill Gorcunov wrote:
> On Wed, Feb 03, 2021 at 03:41:56PM +0300, Pavel Tikhomirov wrote:
>> Currently there is no way to differentiate the file with alive owner
>> from the file with dead owner but pid of the owner reused. That's why
>> CRIU can't actually know if it needs to restore file owner or not,
>> because if it restores owner but actual owner was dead, this can
>> introduce unexpected signals to the "false"-owner (which reused the
>> pid).
>
> Hi! Thanks for the patch. You know I manage to forget the fowner internals.
> Could you please enlighten me -- when owner is set with some pid we do
>
> f_setown_ex
> __f_setown
> f_modown
> filp->f_owner.pid = get_pid(pid);
>
> Thus pid get refcount incremented.

Hi, and yes you are right about refcount is held.

Then the owner exits but refcounter
> should be still up and running and pid should not be reused, no? Or
> I miss something obvious?

AFAICS if pid is held only by 1) fowner refcount and by 2) single
process (without threads, group and session for simplicity), on process
exit we go through:

do_exit
exit_notify
release_task
__exit_signal
__unhash_process
detach_pid
__change_pid
free_pid
idr_remove

So pid is removed from idr, and after that alloc_pid can reuse pid
numbers even if old pid structure is still alive and is still held by
fowner.

Also I've added criu-zdtm test which reproduces the problem:

https://src.openvz.org/projects/OVZ/repos/criu/commits/e25904c35dbc535f6837e55da58ca0f5a5caf4b3#test/zdtm/static/file_fown_reuse.c

Hope this answers your question, Thanks!

>
> The patch itself looks ok on a first glance.
>

--
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.