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

[Pavel Tikhomirov]

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.