Pid namespaces vs locks interaction

Hello!

I am working on pid namespaces vs locks interaction and want to evaluate the idea.
fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if process is belonged to the several namespaces). It is true also for pids in /proc/locks. So correct behavior is saving pointer to the struct pid of the process lock owner.

Signature · 2 lines

Thank,
Vitaliy Gusev

fs/locks.c +1 -0

index 8b8388e..d2d3d75 100644
@@ -125,6 +125,7 @@
 #include <linux/syscalls.h>
 #include <linux/time.h>
 #include <linux/rcupdate.h>
+#include <linux/pid_namespace.h>
 
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>

fs/locks.c +21 -6

 	fl->fl_file = NULL;
 	fl->fl_flags = 0;
 	fl->fl_type = 0;

@@ -553,6 +555,8 @@ static void locks_insert_lock(struct file_lock **pos, struct file_lock *fl)
 {
 	list_add(&fl->fl_link, &file_lock_list);
 
+	fl->fl_nspid = get_pid(task_tgid(current));
+
 	/* insert into file's list */
 	fl->fl_next = *pos;
 	*pos = fl;

@@ -584,6 +588,11 @@ static void locks_delete_lock(struct file_lock **thisfl_p)
 	if (fl->fl_ops && fl->fl_ops->fl_remove)
 		fl->fl_ops->fl_remove(fl);
 
+	if (fl->fl_nspid) {
+		put_pid(fl->fl_nspid);
+		fl->fl_nspid = NULL;
+	}
+
 	locks_wake_up_blocks(fl);
 	locks_free_lock(fl);
 }

@@ -673,14 +682,16 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 		if (posix_locks_conflict(fl, cfl))
 			break;
 	}
-	if (cfl)
+	if (cfl) {
 		__locks_copy_lock(fl, cfl);
-	else
+		if (cfl->fl_nspid)
+			fl->fl_pid = pid_nr_ns(cfl->fl_nspid, 
+						task_active_pid_ns(current));
+	} else
 		fl->fl_type = F_UNLCK;
 	unlock_kernel();
 	return;
 }
-
 EXPORT_SYMBOL(posix_test_lock);
 
 /* This function tests for deadlock condition before putting a process to

@@ -2084,6 +2095,12 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 							int id, char *pfx)
 {
 	struct inode *inode = NULL;
+	unsigned int fl_pid;
+
+	if (fl->fl_nspid)
+		fl_pid = pid_nr_ns(fl->fl_nspid, task_active_pid_ns(current));
+	else
+		fl_pid = fl->fl_pid;
 
 	if (fl->fl_file != NULL)
 		inode = fl->fl_file->f_path.dentry->d_inode;

@@ -2124,16 +2141,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 	}
 	if (inode) {
 #ifdef WE_CAN_BREAK_LSLK_NOW
-		seq_printf(f, "%d %s:%ld ", fl->fl_pid,
+		seq_printf(f, "%d %s:%ld ", fl_pid,
 				inode->i_sb->s_id, inode->i_ino);
 #else
 		/* userspace relies on this representation of dev_t ;-( */
-		seq_printf(f, "%d %02x:%02x:%ld ", fl->fl_pid,
+		seq_printf(f, "%d %02x:%02x:%ld ", fl_pid,
 				MAJOR(inode->i_sb->s_dev),
 				MINOR(inode->i_sb->s_dev), inode->i_ino);
 #endif
 	} else {
-		seq_printf(f, "%d <none>:0 ", fl->fl_pid);
+		seq_printf(f, "%d <none>:0 ", fl_pid);
 	}
 	if (IS_POSIX(fl)) {
 		if (fl->fl_end == OFFSET_MAX)

include/linux/fs.h +1 -0

index b3ec4a4..5876f68 100644
@@ -870,6 +870,7 @@ struct file_lock {
 	struct list_head fl_block;	/* circular list of blocked processes */
 	fl_owner_t fl_owner;
 	unsigned int fl_pid;
+	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
 	unsigned char fl_flags;

Quoted 25 lines · Vitaliy Gusev

Hello!

I am working on pid namespaces vs locks interaction and want to evaluate the idea.
fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if process is belonged to the several namespaces). It is true also for pids in /proc/locks. So correct behavior is saving pointer to the struct pid of the process lock owner.
--
Thank,
Vitaliy Gusev

fs/locks.c +1 -0

index 8b8388e..d2d3d75 100644
@@ -125,6 +125,7 @@
 #include <linux/syscalls.h>
 #include <linux/time.h>
 #include <linux/rcupdate.h>
+#include <linux/pid_namespace.h>
 
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>

fs/locks.c void locks_init_lock(struct file_lock *fl) · line 189

Quoted 4 lines

fs/locks.c +1 -0

@@ -185,6 +186,7 @@ void locks_init_lock(struct file_lock *fl)
 	fl->fl_fasync = NULL;
 	fl->fl_owner = NULL;
 	fl->fl_pid = 0;
+	fl->fl_nspid = NULL;

The idea seems right, but why are you keeping fl->fl_pid around?

Seems like the safer thing to do would be to have a separate struct user_flock, with an integer pid, for communicating to userspace, and a struct flock, with struct pid, for kernel use? Then fcntl_getlk() and fcntl_setlk() do the appropriate conversions.

thanks,

(quoted diff) +0 -2

-serge
-

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 33 lines · On 6 December 2007 17:53:40 Serge E. Hallyn wrote:

Quoted 25 lines · Vitaliy Gusev

Hello!

I am working on pid namespaces vs locks interaction and want to evaluate the idea.
fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if process is belonged to the several namespaces). It is true also for pids in /proc/locks. So correct behavior is saving pointer to the struct pid of the process lock owner.
--
Thank,
Vitaliy Gusev

fs/locks.c +1 -0

index 8b8388e..d2d3d75 100644
@@ -125,6 +125,7 @@
 #include <linux/syscalls.h>
 #include <linux/time.h>
 #include <linux/rcupdate.h>
+#include <linux/pid_namespace.h>
 
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>

fs/locks.c void locks_init_lock(struct file_lock *fl) · line 189

Quoted 4 lines

fs/locks.c +1 -0

@@ -185,6 +186,7 @@ void locks_init_lock(struct file_lock *fl)
 	fl->fl_fasync = NULL;
 	fl->fl_owner = NULL;
 	fl->fl_pid = 0;
+	fl->fl_nspid = NULL;

The idea seems right, but why are you keeping fl->fl_pid around?

Quoted 4 lines

Seems like the safer thing to do would be to have a separate struct user_flock, with an integer pid, for communicating to userspace, and a struct flock, with struct pid, for kernel use? Then fcntl_getlk() and fcntl_setlk() do the appropriate conversions.

fl_pid is used by nfs, fuse and gfs2. For instance nfs keeps in fl_pid some unique id to identify locking process between hosts - it is not a process pid.

Quoted 3 lines · Vitaliy Gusev

fl_pid is used by nfs, fuse and gfs2. For instance nfs keeps in fl_pid some unique id to identify locking process between hosts - it is not a process pid.

Ok, but so the struct user_flock->fl_pid is being set to the task's virtual pid, while the struct kernel_flock->fl_pid is being set to task->tgid for nfsd use.

Why can't nfs just generate a uniqueid from the struct pid when it needs it?

Fuse just seems to copy the pid to report it to userspace, so it would just copy pid_vnr(kernel_flock->pid) into user_flock->fl_pid.

Anyway I haven't looked at all the uses of struct fl_pid, but you can always get the pidnr back from the struct pid if needed so there should be no problem.

Quoted 3 lines

fl_pid is used by nfs, fuse and gfs2. For instance nfs keeps in fl_pid some unique id to identify locking process between hosts - it is not a process pid.

Ok, but so the struct user_flock->fl_pid is being set to the task's virtual pid, while the struct kernel_flock->fl_pid is being set to task->tgid for nfsd use.

Why can't nfs just generate a uniqueid from the struct pid when it needs it?

Fuse just seems to copy the pid to report it to userspace, so it would just copy pid_vnr(kernel_flock->pid) into user_flock->fl_pid.

Anyway I haven't looked at all the uses of struct fl_pid, but you can always get the pidnr back from the struct pid if needed so there should be no problem.

The split definately seems worthwhile to me, so that user_flock->fl_pidnr can always be said to be the pid in the acting process' namespace, and flock->fl_pid can always be a struct pid, rather than having fl_pid sometimes be current->tgid, or sometimes pid_vnr(flock->fl_nspid)...

(quoted diff) +0 -2

-serge
-

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hello

Quoted 8 lines · On 6 December 2007 18:51:30 Serge E. Hallyn wrote:

Quoted 3 lines

fl_pid is used by nfs, fuse and gfs2. For instance nfs keeps in fl_pid some unique id to identify locking process between hosts - it is not a process pid.

Ok, but so the struct user_flock->fl_pid is being set to the task's virtual pid, while the struct kernel_flock->fl_pid is being set to task->tgid for nfsd use.

Quoted 2 lines

Why can't nfs just generate a uniqueid from the struct pid when it needs it?

I think it is hard. lockd uses struct nlm_host to get process unique id (see __nlm_alloc_pid() function).

Quoted 18 lines

Fuse just seems to copy the pid to report it to userspace, so it would just copy pid_vnr(kernel_flock->pid) into user_flock->fl_pid.

Anyway I haven't looked at all the uses of struct fl_pid, but you can always get the pidnr back from the struct pid if needed so there should be no problem.

The split definately seems worthwhile to me, so that user_flock->fl_pidnr can always be said to be the pid in the acting process' namespace, and flock->fl_pid can always be a struct pid, rather than having fl_pid sometimes be current->tgid, or sometimes pid_vnr(flock->fl_nspid)...

(quoted diff) +0 -2

-serge
-

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Thank,
Vitaliy Gusev

Quoted 11 lines · Vitaliy Gusev

Hello

Quoted 8 lines · On 6 December 2007 18:51:30 Serge E. Hallyn wrote:

Quoted 3 lines

fl_pid is used by nfs, fuse and gfs2. For instance nfs keeps in fl_pid some unique id to identify locking process between hosts - it is not a process pid.

Ok, but so the struct user_flock->fl_pid is being set to the task's virtual pid, while the struct kernel_flock->fl_pid is being set to task->tgid for nfsd use.

Quoted 5 lines

Quoted 2 lines

Why can't nfs just generate a uniqueid from the struct pid when it needs it?

I think it is hard. lockd uses struct nlm_host to get process unique id (see __nlm_alloc_pid() function).

Looks pretty simple though... That whole set of code could even stay the same except for in __nlm_alloc_pid():

option 1: compare struct pid* instead of uint32_t pid option 2: use the "global pid" out of the stored struct pid,
something like pid->numbers[0].nr.

Quoted 14 lines

Quoted 14 lines

Fuse just seems to copy the pid to report it to userspace, so it would just copy pid_vnr(kernel_flock->pid) into user_flock->fl_pid.

Anyway I haven't looked at all the uses of struct fl_pid, but you can always get the pidnr back from the struct pid if needed so there should be no problem.

The split definately seems worthwhile to me, so that user_flock->fl_pidnr can always be said to be the pid in the acting process' namespace, and flock->fl_pid can always be a struct pid, rather than having fl_pid sometimes be current->tgid, or sometimes pid_vnr(flock->fl_nspid)...

Quoted 9 lines

Quoted 5 lines

(quoted diff) +0 -2

-serge
-

To unsubscribe from this list: send the line "unsubscribe
linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Thank,
Vitaliy Gusev

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 21 lines · On 12 December 2007 20:31:15 Serge E. Hallyn wrote:

Quoted 11 lines · Vitaliy Gusev

Hello

Quoted 8 lines · On 6 December 2007 18:51:30 Serge E. Hallyn wrote:

Quoted 3 lines

fl_pid is used by nfs, fuse and gfs2. For instance nfs keeps in fl_pid some unique id to identify locking process between hosts - it is not a process pid.

Ok, but so the struct user_flock->fl_pid is being set to the task's virtual pid, while the struct kernel_flock->fl_pid is being set to task->tgid for nfsd use.

Quoted 5 lines

Quoted 2 lines

Why can't nfs just generate a uniqueid from the struct pid when it needs it?

I think it is hard. lockd uses struct nlm_host to get process unique id (see __nlm_alloc_pid() function).

Looks pretty simple though... That whole set of code could even stay the same except for in __nlm_alloc_pid():

Quoted 3 lines

option 1: compare struct pid* instead of uint32_t pid option 2: use the "global pid" out of the stored struct pid,
something like pid->numbers[0].nr.

We can't use process pid. Process pid is circulated! NFS (lockd) needs unique process id between hosts which can't repeat oneself.

Quoted 27 lines

Quoted 13 lines

Quoted 13 lines

Fuse just seems to copy the pid to report it to userspace, so it would just copy pid_vnr(kernel_flock->pid) into user_flock->fl_pid.

Anyway I haven't looked at all the uses of struct fl_pid, but you can always get the pidnr back from the struct pid if needed so there should be no problem.

The split definately seems worthwhile to me, so that user_flock->fl_pidnr can always be said to be the pid in the acting process' namespace, and flock->fl_pid can always be a struct pid, rather than having fl_pid sometimes be current->tgid, or sometimes pid_vnr(flock->fl_nspid)...

Quoted 9 lines

Quoted 5 lines

(quoted diff) +0 -2

-serge
-

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Thank,
Vitaliy Gusev

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Thank,
Vitaliy Gusev

Ok sorry - by letting this thread sit a few days I lost track of where we were.

I see now, so you're saying fl_pid for nfs is not in fact a task pid. It's a magically derived unique id. (And you say it is unique across all the nfs clients?)

So does the p in fl_pid stand for something, or could we rename it to fl_id or fl_uniqueid?

Maybe that's too much bother, but so long as we're bothering with a pid cleanup at all it seems worth it to me. On the other hand maybe J. Bruce Fields was right and we should accept the fact that the flock->fl_pid shouldn't be taken too seriously, and leave it be.

-serge

Quoted 6 lines · On 12 December 2007 21:42:25 Serge E. Hallyn wrote:

Ok sorry - by letting this thread sit a few days I lost track of where we were.

I see now, so you're saying fl_pid for nfs is not in fact a task pid. It's a magically derived unique id. (And you say it is unique across all the nfs clients?)

It is unique for pair client,server.

Quoted 3 lines

So does the p in fl_pid stand for something, or could we rename it to fl_id or fl_uniqueid?

If fl_pid will be renamed with fl_uniqueid or something, it still need accessing from fs/locks.c: cat /proc/locks shows pids which also are NFS pids (unique id).

For example, let's look the /proc/locks in my system (NFS-server) when do flock on a NFS client:

1: POSIX  ADVISORY  WRITE 2 08:06:63116 0 EOF
2: POSIX  ADVISORY  WRITE 7047 08:09:1899694 0 EOF
3: FLOCK  ADVISORY  WRITE 3334 08:06:110497 0 EOF
4: FLOCK  ADVISORY  WRITE 3265 08:06:94786 0 EOF
5: POSIX  ADVISORY  WRITE 2582 08:06:110462 0 EOF

It indicates that process with pid 2 has a posix lock. Really it is a NFS unique id. Problem can be solved by using pid of lockd.

Quoted 4 lines

Maybe that's too much bother, but so long as we're bothering with a pid cleanup at all it seems worth it to me. On the other hand maybe J. Bruce Fields was right and we should accept the fact that the flock->fl_pid shouldn't be taken too seriously, and leave it be.

Mix pids from some namespaces is not good. We can store process pid seen from

init namespace to the flock->fl_pid (instead pid from the current namespace).
Thus fcntl(F_GETLK,...) and "cat /proc/locks" will show global pids. But
some LTP tests can fail.

I am working on pid namespaces vs locks interaction and want to evaluate the idea.
fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if process is belonged to the several namespaces). It is true also for pids in /proc/locks. So correct behavior is saving pointer to the struct pid of the process lock owner.

Quoted 6 lines · Vitaliy Gusev

I am working on pid namespaces vs locks interaction and want to evaluate the idea.
fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if process is belonged to the several namespaces). It is true also for pids in /proc/locks. So correct behavior is saving pointer to the struct pid of the process lock owner.

Forgive me, I'm not familiar with pid namespaces. Exactly what bug does this patch aim to fix?

Quoted 6 lines

fs/locks.c +1 -1

@@ -673,14 +682,16 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 		if (posix_locks_conflict(fl, cfl))
 			break;
 	}
-	if (cfl)
+	if (cfl) {

fs/locks.c posix_test_lock(struct file *filp, struct file_lock *fl) · line 689

Quoted 5 lines

fs/locks.c +3 -1

 		__locks_copy_lock(fl, cfl);
-	else
+		if (cfl->fl_nspid)
+			fl->fl_pid = pid_nr_ns(cfl->fl_nspid, 
+						task_active_pid_ns(current));

What does pid_nr_ns() do? I took a quick look at the implementation and didn't get it.

I tend to think that the pid returned by fcntl(.,F_GETLK,.) shouldn't be taken too seriously--it may be helpful when debugging--e.g. it might help an administrator looking for clues as to who's holding some annoying lock. But it probably shouldn't be depended on for the correctness of an application. Maybe I'm wrong and there's some reason we should worry about it more.

It's also likely to be wrong in the presence of locks held on behalf of nfs clients.

fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if process is belonged to the several namespaces). It is true also for pids in /proc/locks. So correct behavior is saving pointer to the struct pid of the process lock owner.

Assigned-off-by Vitaliy Gusev <[email protected]>

Acked-by Serge Hallyn <[email protected]>

fs/locks.c | 26 +++++++++++++++++++++-----

 include/linux/fs.h |    3 ++-
 2 files changed, 23 insertions(+), 6 deletions(-)

fs/locks.c +21 -5

index 8b8388e..14989fa 100644
@@ -125,6 +125,7 @@
 #include <linux/syscalls.h>
 #include <linux/time.h>
 #include <linux/rcupdate.h>
+#include <linux/pid_namespace.h>
 
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>

@@ -185,6 +186,7 @@ void locks_init_lock(struct file_lock *fl)
 	fl->fl_fasync = NULL;
 	fl->fl_owner = NULL;
 	fl->fl_pid = 0;
+	fl->fl_nspid = NULL;
 	fl->fl_file = NULL;
 	fl->fl_flags = 0;
 	fl->fl_type = 0;

@@ -553,6 +555,8 @@ static void locks_insert_lock(struct file_lock **pos, struct file_lock *fl)
 {
 	list_add(&fl->fl_link, &file_lock_list);
 
+	fl->fl_nspid = get_pid(task_tgid(current));
+
 	/* insert into file's list */
 	fl->fl_next = *pos;
 	*pos = fl;

@@ -584,6 +588,9 @@ static void locks_delete_lock(struct file_lock **thisfl_p)
 	if (fl->fl_ops && fl->fl_ops->fl_remove)
 		fl->fl_ops->fl_remove(fl);
 
+	put_pid(fl->fl_nspid);
+	fl->fl_nspid = NULL;
+
 	locks_wake_up_blocks(fl);
 	locks_free_lock(fl);
 }

@@ -673,9 +680,12 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 		if (posix_locks_conflict(fl, cfl))
 			break;
 	}
-	if (cfl)
+	if (cfl) {
 		__locks_copy_lock(fl, cfl);
-	else
+		if (cfl->fl_nspid)
+			fl->fl_pid = pid_nr_ns(cfl->fl_nspid, 
+						task_active_pid_ns(current));
+	} else
 		fl->fl_type = F_UNLCK;
 	unlock_kernel();
 	return;

@@ -2084,6 +2094,12 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 							int id, char *pfx)
 {
 	struct inode *inode = NULL;
+	unsigned int fl_pid;
+
+	if (fl->fl_nspid)
+		fl_pid = pid_nr_ns(fl->fl_nspid, task_active_pid_ns(current));
+	else
+		fl_pid = fl->fl_pid;
 
 	if (fl->fl_file != NULL)
 		inode = fl->fl_file->f_path.dentry->d_inode;

@@ -2124,16 +2140,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 	}
 	if (inode) {
 #ifdef WE_CAN_BREAK_LSLK_NOW
-		seq_printf(f, "%d %s:%ld ", fl->fl_pid,
+		seq_printf(f, "%d %s:%ld ", fl_pid,
 				inode->i_sb->s_id, inode->i_ino);
 #else
 		/* userspace relies on this representation of dev_t ;-( */
-		seq_printf(f, "%d %02x:%02x:%ld ", fl->fl_pid,
+		seq_printf(f, "%d %02x:%02x:%ld ", fl_pid,
 				MAJOR(inode->i_sb->s_dev),
 				MINOR(inode->i_sb->s_dev), inode->i_ino);
 #endif
 	} else {
-		seq_printf(f, "%d <none>:0 ", fl->fl_pid);
+		seq_printf(f, "%d <none>:0 ", fl_pid);
 	}
 	if (IS_POSIX(fl)) {
 		if (fl->fl_end == OFFSET_MAX)

include/linux/fs.h +2 -1

index b3ec4a4..1fb952f 100644
@@ -869,7 +869,8 @@ struct file_lock {
 	struct list_head fl_link;	/* doubly linked list of all locks */
 	struct list_head fl_block;	/* circular list of blocked processes */
 	fl_owner_t fl_owner;
-	unsigned int fl_pid;
+	unsigned int fl_pid;	/* unique id and sometimes global pid */
+	struct pid *fl_nspid;	/* to calculate owner pid_nr for userspace */
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
 	unsigned char fl_flags;

fs/locks.c +23 -6

index 8b8388e..d2d3d75 100644
@@ -125,6 +125,7 @@
 #include <linux/syscalls.h>
 #include <linux/time.h>
 #include <linux/rcupdate.h>
+#include <linux/pid_namespace.h>
 
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>

@@ -185,6 +186,7 @@ void locks_init_lock(struct file_lock *fl)
 	fl->fl_fasync = NULL;
 	fl->fl_owner = NULL;
 	fl->fl_pid = 0;
+	fl->fl_nspid = NULL;
 	fl->fl_file = NULL;
 	fl->fl_flags = 0;
 	fl->fl_type = 0;

@@ -553,6 +555,8 @@ static void locks_insert_lock(struct file_lock **pos, struct file_lock *fl)
 {
 	list_add(&fl->fl_link, &file_lock_list);
 
+	fl->fl_nspid = get_pid(task_tgid(current));
+
 	/* insert into file's list */
 	fl->fl_next = *pos;
 	*pos = fl;

@@ -584,6 +588,11 @@ static void locks_delete_lock(struct file_lock **thisfl_p)
 	if (fl->fl_ops && fl->fl_ops->fl_remove)
 		fl->fl_ops->fl_remove(fl);
 
+	if (fl->fl_nspid) {
+		put_pid(fl->fl_nspid);
+		fl->fl_nspid = NULL;
+	}
+
 	locks_wake_up_blocks(fl);
 	locks_free_lock(fl);
 }

@@ -673,14 +682,16 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 		if (posix_locks_conflict(fl, cfl))
 			break;
 	}
-	if (cfl)
+	if (cfl) {
 		__locks_copy_lock(fl, cfl);
-	else
+		if (cfl->fl_nspid)
+			fl->fl_pid = pid_nr_ns(cfl->fl_nspid, 
+						task_active_pid_ns(current));
+	} else
 		fl->fl_type = F_UNLCK;
 	unlock_kernel();
 	return;
 }
-
 EXPORT_SYMBOL(posix_test_lock);
 
 /* This function tests for deadlock condition before putting a process to

@@ -2084,6 +2095,12 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 							int id, char *pfx)
 {
 	struct inode *inode = NULL;
+	unsigned int fl_pid;
+
+	if (fl->fl_nspid)
+		fl_pid = pid_nr_ns(fl->fl_nspid, task_active_pid_ns(current));
+	else
+		fl_pid = fl->fl_pid;
 
 	if (fl->fl_file != NULL)
 		inode = fl->fl_file->f_path.dentry->d_inode;

@@ -2124,16 +2141,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
 	}
 	if (inode) {
 #ifdef WE_CAN_BREAK_LSLK_NOW
-		seq_printf(f, "%d %s:%ld ", fl->fl_pid,
+		seq_printf(f, "%d %s:%ld ", fl_pid,
 				inode->i_sb->s_id, inode->i_ino);
 #else
 		/* userspace relies on this representation of dev_t ;-( */
-		seq_printf(f, "%d %02x:%02x:%ld ", fl->fl_pid,
+		seq_printf(f, "%d %02x:%02x:%ld ", fl_pid,
 				MAJOR(inode->i_sb->s_dev),
 				MINOR(inode->i_sb->s_dev), inode->i_ino);
 #endif
 	} else {
-		seq_printf(f, "%d <none>:0 ", fl->fl_pid);
+		seq_printf(f, "%d <none>:0 ", fl_pid);
 	}
 	if (IS_POSIX(fl)) {
 		if (fl->fl_end == OFFSET_MAX)

include/linux/fs.h +1 -0

index b3ec4a4..5876f68 100644
@@ -870,6 +870,7 @@ struct file_lock {
 	struct list_head fl_block;	/* circular list of blocked processes */
 	fl_owner_t fl_owner;
 	unsigned int fl_pid;
+	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
 	struct file *fl_file;
 	unsigned char fl_flags;

fcntl(F_GETLK,..) can return pid of process for not current pid namespace (if process is belonged to the several namespaces). It is true also for pids in /proc/locks. So correct behavior is saving pointer to the struct pid of the process lock owner.

Assigned-off-by Vitaliy Gusev <[email protected]>

[diff-pid-to-nspid.patch text/plain (2.9KB)]