[PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes

[PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes

[Konstantin Khorenko]

The behavior has been changed after 9d5b86ac13c5 ("fs/locks: Remove fl_nspid
and use fs-specific l_pid for remote locks")
and now /proc/$PID/fdinfo/$FD does not show the info about the lock
* if the flock owner process is dead and its pid has been already freed
or
* if the lock owner is not visible in current pidns.

CRIU uses this interface to store locks info during dump and thus can break
on v4.13 and newer.

So let's show info about locks anyway in described cases (like it was before
9d5b86ac13c5), but show pid number saved in file_lock struct if we are in
init_pid_ns (patch 1) or just zero otherwise (patch 2) like we do with SID.

Reproducer:
process A       process A1      process A2
fork()--------->
exit()          open()
                flock()
                fork()--------->
                exit()          sleep()

Before the patch:
================
(root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
pos:    4
flags:  02100002
mnt_id: 257
lock:   (root@vz7)/:

After the patch:
===============
(root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
pos:    4
flags:  02100002
mnt_id: 295
lock:   1: FLOCK  ADVISORY  WRITE ${PID_A1} b6:f8a61:529946 0 EOF

===============
# cat flock1.c

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/file.h>
#include <fcntl.h>

int main(void)
{
        int fd;
        int err;
        pid_t child_pid;

        child_pid = fork();
        if (child_pid == -1)
                perror("fork failed");
        if (child_pid) {
                exit(0);
        }

        fd = open("/tmp/a", O_CREAT | O_RDWR);
        if (fd == -1)
                perror("Failed to open the file");

        err = flock(fd, LOCK_EX);
        if (err == -1)
                perror("flock failed");

        child_pid = fork();
        if (child_pid == -1)
                perror("fork failed");
        if (child_pid)
                exit(0);

        sleep(10000);

        return 0;
}

Konstantin Khorenko (2):
  fs/lock: skip lock owner pid translation in case we are in init_pid_ns
  fs/lock: show locks taken by processes from another pidns

 fs/locks.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

-- 
2.15.1

[PATCH 1/2] fs/lock: skip lock owner pid translation in case we are in init_pid_ns

[Konstantin Khorenko]

If the flock owner process is dead and its pid has been already freed,
pid translation won't work, but we still want to show flock owner pid
number when expecting /proc/$PID/fdinfo/$FD in init pidns.

Reproducer:
process A	process A1	process A2
fork()--------->
exit()		open()
		flock()
		fork()--------->
		exit()		sleep()

Before the patch:
================
(root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
pos:    4
flags:  02100002
mnt_id: 257
lock:   (root@vz7)/:

After the patch:
===============
(root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
pos:    4
flags:  02100002
mnt_id: 295
lock:   1: FLOCK  ADVISORY  WRITE ${PID_A1} b6:f8a61:529946 0 EOF

Fixes: 9d5b86ac13c5 ("fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks")
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/locks.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/locks.c b/fs/locks.c
index 05e211be8684..bfee5b7f2862 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2072,6 +2072,13 @@ static pid_t locks_translate_pid(struct file_lock *fl, struct pid_namespace *ns)
 		return -1;
 	if (IS_REMOTELCK(fl))
 		return fl->fl_pid;
+	/*
+	 * If the flock owner process is dead and its pid has been already
+	 * freed, the translation below won't work, but we still want to show
+	 * flock owner pid number in init pidns.
+	 */
+	if (ns == &init_pid_ns)
+		return (pid_t)fl->fl_pid;
 
 	rcu_read_lock();
 	pid = find_pid_ns(fl->fl_pid, &init_pid_ns);
-- 
2.15.1

Re: [PATCH 1/2] fs/lock: skip lock owner pid translation in case we are in init_pid_ns

[Andrey Vagin]

On Fri, Jun 08, 2018 at 05:27:11PM +0300, Konstantin Khorenko wrote:
> If the flock owner process is dead and its pid has been already freed,
> pid translation won't work, but we still want to show flock owner pid
> number when expecting /proc/$PID/fdinfo/$FD in init pidns.
> 
> Reproducer:
> process A	process A1	process A2
> fork()--------->
> exit()		open()
> 		flock()
> 		fork()--------->
> 		exit()		sleep()
> 
> Before the patch:
> ================
> (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 257
> lock:   (root@vz7)/:
> 
> After the patch:
> ===============
> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 295
> lock:   1: FLOCK  ADVISORY  WRITE ${PID_A1} b6:f8a61:529946 0 EOF
> 

Acked-by: Andrey Vagin <avagin@openvz.org>

> Fixes: 9d5b86ac13c5 ("fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks")
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>  fs/locks.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 05e211be8684..bfee5b7f2862 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2072,6 +2072,13 @@ static pid_t locks_translate_pid(struct file_lock *fl, struct pid_namespace *ns)
>  		return -1;
>  	if (IS_REMOTELCK(fl))
>  		return fl->fl_pid;
> +	/*
> +	 * If the flock owner process is dead and its pid has been already
> +	 * freed, the translation below won't work, but we still want to show
> +	 * flock owner pid number in init pidns.
> +	 */
> +	if (ns == &init_pid_ns)
> +		return (pid_t)fl->fl_pid;
>  
>  	rcu_read_lock();
>  	pid = find_pid_ns(fl->fl_pid, &init_pid_ns);
> -- 
> 2.15.1
> 

[PATCH 2/2] fs/lock: show locks taken by processes from another pidns

[Konstantin Khorenko]

Currently if we face a lock taken by a process invisible in the current
pidns we skip the lock completely, but this

1) makes the output not that nice
    (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
    pos:    4
    flags:  02100002
    mnt_id: 257
    lock:   (root@vz7)/:

2) makes it more difficult to debug issues with leaked flocks
   if you get error on lock, but don't see any locks in /proc/$id/fdinfo/$file

Let's show information about such locks again as previously, but
show zero in the owner pid field.

After the patch:
===============
(root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
pos:    4
flags:  02100002
mnt_id: 295
lock:   1: FLOCK  ADVISORY  WRITE 0 b6:f8a61:529946 0 EOF

Fixes: 9d5b86ac13c5 ("fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks")
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/locks.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index bfee5b7f2862..e533623e2e99 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2633,12 +2633,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 
 	fl_pid = locks_translate_pid(fl, proc_pidns);
 	/*
-	 * If there isn't a fl_pid don't display who is waiting on
-	 * the lock if we are called from locks_show, or if we are
-	 * called from __show_fd_info - skip lock entirely
+	 * If lock owner is dead (and pid is freed) or not visible in current
+	 * pidns, zero is shown as a pid value. Check lock info from
+	 * init_pid_ns to get saved lock pid value.
 	 */
-	if (fl_pid == 0)
-		return;
 
 	if (fl->fl_file != NULL)
 		inode = locks_inode(fl->fl_file);
-- 
2.15.1

Re: [PATCH 2/2] fs/lock: show locks taken by processes from another pidns

[Andrey Vagin]

On Fri, Jun 08, 2018 at 05:27:12PM +0300, Konstantin Khorenko wrote:
> Currently if we face a lock taken by a process invisible in the current
> pidns we skip the lock completely, but this
> 
> 1) makes the output not that nice
>     (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
>     pos:    4
>     flags:  02100002
>     mnt_id: 257
>     lock:   (root@vz7)/:
> 
> 2) makes it more difficult to debug issues with leaked flocks
>    if you get error on lock, but don't see any locks in /proc/$id/fdinfo/$file

3) breaks the CRIU project. criu reads fdinfo to dump file locks.

> 
> Let's show information about such locks again as previously, but
> show zero in the owner pid field.
> 
> After the patch:
> ===============
> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 295
> lock:   1: FLOCK  ADVISORY  WRITE 0 b6:f8a61:529946 0 EOF
>

I think initially this was broken in this commit:

Fixes: d67fd44f697d ("locks: Filter /proc/locks output on proc pid ns")


Acked-by: Andrei Vagin <avagin@openvz.org>

Thanks,
Andrei

> Fixes: 9d5b86ac13c5 ("fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks")
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>  fs/locks.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index bfee5b7f2862..e533623e2e99 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2633,12 +2633,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
>  
>  	fl_pid = locks_translate_pid(fl, proc_pidns);
>  	/*
> -	 * If there isn't a fl_pid don't display who is waiting on
> -	 * the lock if we are called from locks_show, or if we are
> -	 * called from __show_fd_info - skip lock entirely
> +	 * If lock owner is dead (and pid is freed) or not visible in current
> +	 * pidns, zero is shown as a pid value. Check lock info from
> +	 * init_pid_ns to get saved lock pid value.
>  	 */
> -	if (fl_pid == 0)
> -		return;
>  
>  	if (fl->fl_file != NULL)
>  		inode = locks_inode(fl->fl_file);
> -- 
> 2.15.1
> 

Re: [PATCH 2/2] fs/lock: show locks taken by processes from another pidns

[Jeff Layton]

On Fri, 2018-06-08 at 17:27 +0300, Konstantin Khorenko wrote:
> Currently if we face a lock taken by a process invisible in the current
> pidns we skip the lock completely, but this
> 
> 1) makes the output not that nice
>     (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
>     pos:    4
>     flags:  02100002
>     mnt_id: 257
>     lock:   (root@vz7)/:
> 
> 2) makes it more difficult to debug issues with leaked flocks
>    if you get error on lock, but don't see any locks in /proc/$id/fdinfo/$file
> 
> Let's show information about such locks again as previously, but
> show zero in the owner pid field.
> 
> After the patch:
> ===============
> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 295
> lock:   1: FLOCK  ADVISORY  WRITE 0 b6:f8a61:529946 0 EOF
> 
> Fixes: 9d5b86ac13c5 ("fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks")
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>  fs/locks.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index bfee5b7f2862..e533623e2e99 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2633,12 +2633,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
>  
>  	fl_pid = locks_translate_pid(fl, proc_pidns);
>  	/*
> -	 * If there isn't a fl_pid don't display who is waiting on
> -	 * the lock if we are called from locks_show, or if we are
> -	 * called from __show_fd_info - skip lock entirely
> +	 * If lock owner is dead (and pid is freed) or not visible in current
> +	 * pidns, zero is shown as a pid value. Check lock info from
> +	 * init_pid_ns to get saved lock pid value.
>  	 */
> -	if (fl_pid == 0)
> -		return;
>  
>  	if (fl->fl_file != NULL)
>  		inode = locks_inode(fl->fl_file);

(cc'ing Nickolay)

As Andrey points out, this behavior was originally added in commit
d67fd44f697d to address performance issues when there are a lot of locks
held by tasks in other namespaces.

Will allowing this code to show these again cause a problem there?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 2/2] fs/lock: show locks taken by processes from another pidns

[Andrey Vagin]

On Thu, Jun 14, 2018 at 07:00:07AM -0400, Jeff Layton wrote:
> On Fri, 2018-06-08 at 17:27 +0300, Konstantin Khorenko wrote:
> > Currently if we face a lock taken by a process invisible in the current
> > pidns we skip the lock completely, but this
> > 
> > 1) makes the output not that nice
> >     (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
> >     pos:    4
> >     flags:  02100002
> >     mnt_id: 257
> >     lock:   (root@vz7)/:
> > 
> > 2) makes it more difficult to debug issues with leaked flocks
> >    if you get error on lock, but don't see any locks in /proc/$id/fdinfo/$file
> > 
> > Let's show information about such locks again as previously, but
> > show zero in the owner pid field.
> > 
> > After the patch:
> > ===============
> > (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> > pos:    4
> > flags:  02100002
> > mnt_id: 295
> > lock:   1: FLOCK  ADVISORY  WRITE 0 b6:f8a61:529946 0 EOF
> > 
> > Fixes: 9d5b86ac13c5 ("fs/locks: Remove fl_nspid and use fs-specific l_pid for remote locks")
> > Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> > ---
> >  fs/locks.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/locks.c b/fs/locks.c
> > index bfee5b7f2862..e533623e2e99 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -2633,12 +2633,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
> >  
> >  	fl_pid = locks_translate_pid(fl, proc_pidns);
> >  	/*
> > -	 * If there isn't a fl_pid don't display who is waiting on
> > -	 * the lock if we are called from locks_show, or if we are
> > -	 * called from __show_fd_info - skip lock entirely
> > +	 * If lock owner is dead (and pid is freed) or not visible in current
> > +	 * pidns, zero is shown as a pid value. Check lock info from
> > +	 * init_pid_ns to get saved lock pid value.
> >  	 */
> > -	if (fl_pid == 0)
> > -		return;
> >  
> >  	if (fl->fl_file != NULL)
> >  		inode = locks_inode(fl->fl_file);
> 
> (cc'ing Nickolay)
> 
> As Andrey points out, this behavior was originally added in commit
> d67fd44f697d to address performance issues when there are a lot of locks
> held by tasks in other namespaces.
> 
> Will allowing this code to show these again cause a problem there?

No, it will not. The content of /proc/locks is still filtered. As for
fdinfo, it shows locks for one file descriptor, there will not be a lot of
locks. And fdinfo was designed to show all locks for a file descriptor,
it doesn't matter in what pidns they were taken.

Thanks,
Andrei

> -- 
> Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes

[Cyrill Gorcunov]

On Fri, Jun 08, 2018 at 05:27:10PM +0300, Konstantin Khorenko wrote:
> The behavior has been changed after 9d5b86ac13c5 ("fs/locks: Remove fl_nspid
> and use fs-specific l_pid for remote locks")
> and now /proc/$PID/fdinfo/$FD does not show the info about the lock
> * if the flock owner process is dead and its pid has been already freed
> or
> * if the lock owner is not visible in current pidns.
> 
> CRIU uses this interface to store locks info during dump and thus can break
> on v4.13 and newer.

Indeed, it should cause problems with procfs parsing. We should add a test
for such scenarions, and since we're running tests by linux-next all the time
we would have this issue catched already.

The series looks ok to me, but I'm not flock expert so better wait for more
trustworthy opinion.

> 
> So let's show info about locks anyway in described cases (like it was before
> 9d5b86ac13c5), but show pid number saved in file_lock struct if we are in
> init_pid_ns (patch 1) or just zero otherwise (patch 2) like we do with SID.

Re: [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes

[Jeff Layton]

On Fri, 2018-06-08 at 17:27 +0300, Konstantin Khorenko wrote:
> The behavior has been changed after 9d5b86ac13c5 ("fs/locks: Remove fl_nspid
> and use fs-specific l_pid for remote locks")
> and now /proc/$PID/fdinfo/$FD does not show the info about the lock
> * if the flock owner process is dead and its pid has been already freed
> or
> * if the lock owner is not visible in current pidns.
> 
> CRIU uses this interface to store locks info during dump and thus can break
> on v4.13 and newer.
> 
> So let's show info about locks anyway in described cases (like it was before
> 9d5b86ac13c5), but show pid number saved in file_lock struct if we are in
> init_pid_ns (patch 1) or just zero otherwise (patch 2) like we do with SID.
> 
> Reproducer:
> process A       process A1      process A2
> fork()--------->
> exit()          open()
>                 flock()
>                 fork()--------->
>                 exit()          sleep()
> 
> Before the patch:
> ================
> (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 257
> lock:   (root@vz7)/:
> 
> After the patch:
> ===============
> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 295
> lock:   1: FLOCK  ADVISORY  WRITE ${PID_A1} b6:f8a61:529946 0 EOF
> 
> ===============
> # cat flock1.c
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/file.h>
> #include <fcntl.h>
> 
> int main(void)
> {
>         int fd;
>         int err;
>         pid_t child_pid;
> 
>         child_pid = fork();
>         if (child_pid == -1)
>                 perror("fork failed");
>         if (child_pid) {
>                 exit(0);
>         }
> 
>         fd = open("/tmp/a", O_CREAT | O_RDWR);
>         if (fd == -1)
>                 perror("Failed to open the file");
> 
>         err = flock(fd, LOCK_EX);
>         if (err == -1)
>                 perror("flock failed");
> 
>         child_pid = fork();
>         if (child_pid == -1)
>                 perror("fork failed");
>         if (child_pid)
>                 exit(0);
> 
>         sleep(10000);
> 
>         return 0;
> }
> 
> Konstantin Khorenko (2):
>   fs/lock: skip lock owner pid translation in case we are in init_pid_ns
>   fs/lock: show locks taken by processes from another pidns
> 
>  fs/locks.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 

These look fine to me. I'll plan to pick them up for v4.19 unless anyone
has objections.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes

[Benjamin Coddington]

On 8 Jun 2018, at 10:27, Konstantin Khorenko wrote:

> The behavior has been changed after 9d5b86ac13c5 ("fs/locks: Remove 
> fl_nspid
> and use fs-specific l_pid for remote locks")
> and now /proc/$PID/fdinfo/$FD does not show the info about the lock
> * if the flock owner process is dead and its pid has been already 
> freed
> or
> * if the lock owner is not visible in current pidns.
>
> CRIU uses this interface to store locks info during dump and thus can 
> break
> on v4.13 and newer.
>
> So let's show info about locks anyway in described cases (like it was 
> before
> 9d5b86ac13c5), but show pid number saved in file_lock struct if we are 
> in
> init_pid_ns (patch 1) or just zero otherwise (patch 2) like we do with 
> SID.
>
> Reproducer:
> process A       process A1      process A2
> fork()--------->
> exit()          open()
>                 flock()
>                 fork()--------->
>                 exit()          sleep()
>
> Before the patch:
> ================
> (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 257
> lock:   (root@vz7)/:
>
> After the patch:
> ===============
> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 295
> lock:   1: FLOCK  ADVISORY  WRITE ${PID_A1} b6:f8a61:529946 0 EOF
>
> ===============
> # cat flock1.c
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/file.h>
> #include <fcntl.h>
>
> int main(void)
> {
>         int fd;
>         int err;
>         pid_t child_pid;
>
>         child_pid = fork();
>         if (child_pid == -1)
>                 perror("fork failed");
>         if (child_pid) {
>                 exit(0);
>         }
>
>         fd = open("/tmp/a", O_CREAT | O_RDWR);
>         if (fd == -1)
>                 perror("Failed to open the file");
>
>         err = flock(fd, LOCK_EX);
>         if (err == -1)
>                 perror("flock failed");
>
>         child_pid = fork();
>         if (child_pid == -1)
>                 perror("fork failed");
>         if (child_pid)
>                 exit(0);
>
>         sleep(10000);
>
>         return 0;
> }
>
> Konstantin Khorenko (2):
>   fs/lock: skip lock owner pid translation in case we are in 
> init_pid_ns
>   fs/lock: show locks taken by processes from another pidns


These look good to me too.  It makes sense that we treat pid numbers out 
of
our namespace in the same way we'd treat remote pids, by returning 0 
instead
of skipping their display altogether.  You can add my

Reviewed-by: Benjamin Coddington <bcodding@redhat.com>

to these two.

Ben

Re: [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes

[Murphy Zhou]

Hi,

Looks like this missed v4.18 ?

Thanks,
Murphy

On Fri, Jun 8, 2018 at 10:27 PM, Konstantin Khorenko
<khorenko@virtuozzo.com> wrote:
> The behavior has been changed after 9d5b86ac13c5 ("fs/locks: Remove fl_nspid
> and use fs-specific l_pid for remote locks")
> and now /proc/$PID/fdinfo/$FD does not show the info about the lock
> * if the flock owner process is dead and its pid has been already freed
> or
> * if the lock owner is not visible in current pidns.
>
> CRIU uses this interface to store locks info during dump and thus can break
> on v4.13 and newer.
>
> So let's show info about locks anyway in described cases (like it was before
> 9d5b86ac13c5), but show pid number saved in file_lock struct if we are in
> init_pid_ns (patch 1) or just zero otherwise (patch 2) like we do with SID.
>
> Reproducer:
> process A       process A1      process A2
> fork()--------->
> exit()          open()
>                 flock()
>                 fork()--------->
>                 exit()          sleep()
>
> Before the patch:
> ================
> (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 257
> lock:   (root@vz7)/:
>
> After the patch:
> ===============
> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
> pos:    4
> flags:  02100002
> mnt_id: 295
> lock:   1: FLOCK  ADVISORY  WRITE ${PID_A1} b6:f8a61:529946 0 EOF
>
> ===============
> # cat flock1.c
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/file.h>
> #include <fcntl.h>
>
> int main(void)
> {
>         int fd;
>         int err;
>         pid_t child_pid;
>
>         child_pid = fork();
>         if (child_pid == -1)
>                 perror("fork failed");
>         if (child_pid) {
>                 exit(0);
>         }
>
>         fd = open("/tmp/a", O_CREAT | O_RDWR);
>         if (fd == -1)
>                 perror("Failed to open the file");
>
>         err = flock(fd, LOCK_EX);
>         if (err == -1)
>                 perror("flock failed");
>
>         child_pid = fork();
>         if (child_pid == -1)
>                 perror("fork failed");
>         if (child_pid)
>                 exit(0);
>
>         sleep(10000);
>
>         return 0;
> }
>
> Konstantin Khorenko (2):
>   fs/lock: skip lock owner pid translation in case we are in init_pid_ns
>   fs/lock: show locks taken by processes from another pidns
>
>  fs/locks.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> --
> 2.15.1
>

Re: [PATCH 0/2] fs/lock: show locks info owned by dead/invisible processes

[Konstantin Khorenko]

On 08/09/2018 10:16 AM, Murphy Zhou wrote:
> Hi,
>
> Looks like this missed v4.18 ?

Hi Murphy,

yes, Jeff planned to push those patches into 4.19 and they are in "linux-next" now,
but not in 4.18 "master" currently.

On 06/14/2018 01:41 PM, Jeff Layton wrote:
 > These look fine to me. I'll plan to pick them up for v4.19 unless anyone
 > has objections.

linux-next:
1cf8e5de4055 ("fs/lock: show locks taken by processes from another pidns")
826d7bc9f013 ("fs/lock: skip lock owner pid translation in case we are in init_pid_ns")

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

> On Fri, Jun 8, 2018 at 10:27 PM, Konstantin Khorenko
> <khorenko@virtuozzo.com> wrote:
>> The behavior has been changed after 9d5b86ac13c5 ("fs/locks: Remove fl_nspid
>> and use fs-specific l_pid for remote locks")
>> and now /proc/$PID/fdinfo/$FD does not show the info about the lock
>> * if the flock owner process is dead and its pid has been already freed
>> or
>> * if the lock owner is not visible in current pidns.
>>
>> CRIU uses this interface to store locks info during dump and thus can break
>> on v4.13 and newer.
>>
>> So let's show info about locks anyway in described cases (like it was before
>> 9d5b86ac13c5), but show pid number saved in file_lock struct if we are in
>> init_pid_ns (patch 1) or just zero otherwise (patch 2) like we do with SID.
>>
>> Reproducer:
>> process A       process A1      process A2
>> fork()--------->
>> exit()          open()
>>                 flock()
>>                 fork()--------->
>>                 exit()          sleep()
>>
>> Before the patch:
>> ================
>> (root@vz7)/: cat /proc/${PID_A2}/fdinfo/3
>> pos:    4
>> flags:  02100002
>> mnt_id: 257
>> lock:   (root@vz7)/:
>>
>> After the patch:
>> ===============
>> (root@vz7)/:cat /proc/${PID_A2}/fdinfo/3
>> pos:    4
>> flags:  02100002
>> mnt_id: 295
>> lock:   1: FLOCK  ADVISORY  WRITE ${PID_A1} b6:f8a61:529946 0 EOF
>>
>> ===============
>> # cat flock1.c
>>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <unistd.h>
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <sys/file.h>
>> #include <fcntl.h>
>>
>> int main(void)
>> {
>>         int fd;
>>         int err;
>>         pid_t child_pid;
>>
>>         child_pid = fork();
>>         if (child_pid == -1)
>>                 perror("fork failed");
>>         if (child_pid) {
>>                 exit(0);
>>         }
>>
>>         fd = open("/tmp/a", O_CREAT | O_RDWR);
>>         if (fd == -1)
>>                 perror("Failed to open the file");
>>
>>         err = flock(fd, LOCK_EX);
>>         if (err == -1)
>>                 perror("flock failed");
>>
>>         child_pid = fork();
>>         if (child_pid == -1)
>>                 perror("fork failed");
>>         if (child_pid)
>>                 exit(0);
>>
>>         sleep(10000);
>>
>>         return 0;
>> }
>>
>> Konstantin Khorenko (2):
>>   fs/lock: skip lock owner pid translation in case we are in init_pid_ns
>>   fs/lock: show locks taken by processes from another pidns
>>
>>  fs/locks.c | 15 ++++++++++-----
>>  1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> --
>> 2.15.1
>>
> .
>