Re: [PATCHv3] locks: Filter /proc/locks output on proc pid ns

[ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman)]

ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) writes:

> Nikolay Borisov <n.borisov.lkml-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> On  4.08.2016 00:09, Eric W. Biederman wrote:
>>> Dumb question does anyone know the difference between fl_nspid and
>>> fl_pid off the top of your heads?  I am looking at the code and I am
>>> confused why we have to both.  I am afraid that there was some
>>> sloppiness when the pid namespace was implemented and this was the
>>> result.  I remember that file locks were a rough spot during the
>>> conversion but I don't recall the details off the top of my head.
>>
>> I think the fl_nspid is the actual pid namespace where the lock was
>> created, where as pid can be just a unique value required by NFS. I
>> gathered that information while reading the changelogs and the mailing
>> list discussion here:
>> https://lists.linuxfoundation.org/pipermail/containers/2007-December/009044.html
>
> Thanks for the old thread.
>
> Researching myself I see that for posix locks we have struct flock
> that has a field l_pid that must be the pid of the process holding
> the lock.
>
> It looks like the explanation given in the old thread was inadequate,
> and the code in the kernel is definitely incorrect with respect to pid
> namespaces.  If the process holding the lock is in a different pid
> namespace than the process waiting for the lock l_pid will be set
> incorrectly.
>
> Looking at the code it appears from the perspective of struct file_lock
> the only difference between a remote lock and a local lock is that
> fl_owner is set to point at a different structure.
>
> Looking at the nfs case if I am reading my sources correctly the struct
> nlm field named svid is the process id on the remote system, and other
> nlm fields distinguish the host.
>
> Is the desired behavior for nfs that for a remote lock we set l_pid
> to the remote process id, and don't report a hint about which machine
> the process id is on?
>
> It does seem to make sense to have both fl_pid as a value usable
> remotely and otherwise and fl_nspid (arguably it should be fl_local_pid)
> as a value usable on the local machine.
>
> I think the code that sets fl_nspid today is anything but obviously
> correct and probably needs to be rewritten.

Bah.  I take it back F_GETFL isis not broken when crossing pid
namespaces, the code is just dangerously clever.

Eric