Skip to contentSkip to conversation

pidns: Fix a leak in /proc inodes and dentries

PATCH In review Sukadev Bhattiprolu 6 participants 6 messages Oct 20, 2009 1 message not in the archive Proc filesystem

lore
Share & tools
lore

Sukadev Bhattiprolu

author
#0 lore raw

PATCH 1/1 pidns: Fix a leak in /proc inodes and dentries

Fix a leak in /proc dentries and inodes with pid namespaces.

This fix reverts the commit 7766755a2f249e7e0. The leak was reported by Daniel Lezcano - see http://lkml.org/lkml/2009/10/2/159.

To summarize the thread, when container-init is terminated, it sets the PF_EXITING flag and then zaps all the other processes in the container. When those processes exit, they are expected to be reaped by the container- init and as a part of reaping, the container-init should flush any /proc dentries associated with the processes. But because the container-init is itself exiting and the following PF_EXITING check, the dentires are not flushed, resulting in leak in /proc inodes and dentries.

Signed-off-by Sukadev Bhattiprolu <[email protected]>
Reported-by Daniel Lezcano <[email protected]>
Cc Eric W. Biederman <[email protected]>
Cc Jan Kara <[email protected]>
Cc Andrea Arcangeli <[email protected]>
---
 fs/proc/base.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
fs/proc/base.c +1 -2
Index: linux-2.6/fs/proc/base.c
===================================================================
@@ -2580,8 +2580,7 @@ static void proc_flush_task_mnt(struct v
 	name.len = snprintf(buf, sizeof(buf), "%d", pid);
 	dentry = d_hash_and_lookup(mnt->mnt_root, &name);
 	if (dentry) {
-		if (!(current->flags & PF_EXITING))
-			shrink_dcache_parent(dentry);
+		shrink_dcache_parent(dentry);
 		d_drop(dentry);
 		dput(dentry);
 	}

A message that is not in the archive (sent off-list or not yet received): 20091020041337.GA31623-r/[email protected]

Acked-by
#2 lore raw
Quoted 5 lines · Sukadev Bhattiprolu

Fix a leak in /proc dentries and inodes with pid namespaces.

This fix reverts the commit 7766755a2f249e7e0. The leak was reported by Daniel Lezcano - see http://lkml.org/lkml/2009/10/2/159.

Quoted 7 lines

To summarize the thread, when container-init is terminated, it sets the PF_EXITING flag and then zaps all the other processes in the container. When those processes exit, they are expected to be reaped by the container- init and as a part of reaping, the container-init should flush any /proc dentries associated with the processes. But because the container-init is itself exiting and the following PF_EXITING check, the dentires are not flushed, resulting in leak in /proc inodes and dentries.

Acked-by "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
Quoted 24 lines
Signed-off-by Sukadev Bhattiprolu <sukadev-r/[email protected]>
Reported-by Daniel Lezcano <dlezcano-NmTC/[email protected]>
Cc Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>
Cc Jan Kara <[email protected]>
Cc Andrea Arcangeli <andrea-Vyt77T80VFVWk0Htik3J/[email protected]>
---
 fs/proc/base.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6/fs/proc/base.c
===================================================================

fs/proc/base.c +1 -2
@@ -2580,8 +2580,7 @@ static void proc_flush_task_mnt(struct v
 	name.len = snprintf(buf, sizeof(buf), "%d", pid);
 	dentry = d_hash_and_lookup(mnt->mnt_root, &name);
 	if (dentry) {
-		if (!(current->flags & PF_EXITING))
-			shrink_dcache_parent(dentry);
+		shrink_dcache_parent(dentry);
 		d_drop(dentry);
 		dput(dentry);
 	}
#3 lore raw
In reply to a message not in the archive

fs/proc/base.c static void proc_flush_task_mnt(struct v · line 2583

Quoted 9 lines · Sukadev Bhattiprolu
fs/proc/base.c +1 -2
@@ -2580,8 +2580,7 @@ static void proc_flush_task_mnt(struct v
 	name.len = snprintf(buf, sizeof(buf), "%d", pid);
 	dentry = d_hash_and_lookup(mnt->mnt_root, &name);
 	if (dentry) {
-		if (!(current->flags & PF_EXITING))
-			shrink_dcache_parent(dentry);
+		shrink_dcache_parent(dentry);

What to do with original deadlock?

Acked-by on v1 1/1
#4 lore raw
In reply to Sukadev Bhattiprolu
Quoted 5 lines · Sukadev Bhattiprolu

Fix a leak in /proc dentries and inodes with pid namespaces.

This fix reverts the commit 7766755a2f249e7e0. The leak was reported by Daniel Lezcano - see http://lkml.org/lkml/2009/10/2/159.

Quoted 7 lines

To summarize the thread, when container-init is terminated, it sets the PF_EXITING flag and then zaps all the other processes in the container. When those processes exit, they are expected to be reaped by the container- init and as a part of reaping, the container-init should flush any /proc dentries associated with the processes. But because the container-init is itself exiting and the following PF_EXITING check, the dentires are not flushed, resulting in leak in /proc inodes and dentries.

Acked-by "Eric W. Biederman" <[email protected]>
Quoted 24 lines
Signed-off-by Sukadev Bhattiprolu <[email protected]>
Reported-by Daniel Lezcano <[email protected]>
Cc Eric W. Biederman <[email protected]>
Cc Jan Kara <[email protected]>
Cc Andrea Arcangeli <[email protected]>
---
 fs/proc/base.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6/fs/proc/base.c
===================================================================

fs/proc/base.c +1 -2
@@ -2580,8 +2580,7 @@ static void proc_flush_task_mnt(struct v
 	name.len = snprintf(buf, sizeof(buf), "%d", pid);
 	dentry = d_hash_and_lookup(mnt->mnt_root, &name);
 	if (dentry) {
-		if (!(current->flags & PF_EXITING))
-			shrink_dcache_parent(dentry);
+		shrink_dcache_parent(dentry);
 		d_drop(dentry);
 		dput(dentry);
 	}
#5 lore raw
Quoted 15 lines · Eric W. Biederman
Quoted 5 lines · Sukadev Bhattiprolu

Fix a leak in /proc dentries and inodes with pid namespaces.

This fix reverts the commit 7766755a2f249e7e0. The leak was reported by Daniel Lezcano - see http://lkml.org/lkml/2009/10/2/159.

Quoted 7 lines

To summarize the thread, when container-init is terminated, it sets the PF_EXITING flag and then zaps all the other processes in the container. When those processes exit, they are expected to be reaped by the container- init and as a part of reaping, the container-init should flush any /proc dentries associated with the processes. But because the container-init is itself exiting and the following PF_EXITING check, the dentires are not flushed, resulting in leak in /proc inodes and dentries.

Quoted 1 line
Acked-by "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>

there's indeed a lot of progress. A run of our C/R testsuite (spawning and killing a few thousands pidns) used to leak ~700MB of slab, it's now gone. thanks suka for spending time on this.

but we still have some minor leaks. below are contents of /proc/slabinfo before and after the run, you will notice that in some cases, dangling refs of nsproxy and pid_namespace are still alive. I wonder if there are some cases when this can happen, else I'll try to reproduce it.

Cheers,

C.

* slabinfo.qemu (i686)

pid_namespace          0      0     64   59    1 
nsproxy                0      0     48   78    1 
proc_inode_cache     193    193   4096    1    1 
dentry              6734   6734   4096    1    1 
pid_2                  0      0     88   44    1 
pid_namespace          0      0     64   59    1 
nsproxy                0      0     48   78    1 
proc_inode_cache       4      4   4096    1    1 
dentry             36112  36112   4096    1    1

* slabinfo.a13.test.meiosys.com (ppc64)

pid_namespace          0      0   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     506    513   4096    1    1 
dentry              6269   6272    280   14    1 
pid_2                  1     28    136   28    1 
pid_namespace          1      1   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     486    498   4096    1    1 
dentry             49051  49448    280   14    1

* slabinfo.f13.test.meiosys.com (ppc64)

pid_namespace          0      0   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     248    263   4096    1    1 
dentry              7359   7364    280   14    1 
pid_2                  0      0    136   28    1 
pid_namespace          0      0   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     240    240   4096    1    1 
dentry             50253  50666    280   14    1

* slabinfo.r3-23.test.meiosys.com (x86_64)

pid_namespace          0      0   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     479    495   4096    1    1 
dentry              5614   5614    280   14    1 
pid_2                  1     28    136   28    1 
pid_namespace          1      1   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     576    576   4096    1    1 
dentry             52202  52444    280   14    1

* slabinfo.r3-24.test.meiosys.com (x86_64)

pid_namespace          0      0   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     464    486   4096    1    1 
dentry              5698   5698    280   14    1 
pid_2                  0      0    136   28    1 
pid_namespace          0      0   4096    1    1 
nsproxy                0      0     72   53    1 
proc_inode_cache     448    448   4096    1    1 
dentry             51845  52164    280   14    1

* slabinfo.r3-26.test.meiosys.com (i686)

pid_namespace          0      0     64   59    1 
nsproxy                0      0     48   78    1 
proc_inode_cache     449    466   4096    1    1 
dentry              5633   5633   4096    1    1 
pid_2                  0      0     88   44    1 
pid_namespace          0      0     64   59    1 
nsproxy                0      0     48   78    1 
proc_inode_cache     448    448   4096    1    1 
dentry             52039  52039   4096    1    1

* slabinfo.linuz12 (s390x)

pid_namespace          0      0   2112    3    2 
nsproxy                0      0     48   77    1 
proc_inode_cache     725    798    640    6    1 
dentry              4340   4340    192   20    1 
pid_2                  1     30    128   30    1 
pid_namespace          1      3   2112    3    2 
nsproxy                1     77     48   77    1 
proc_inode_cache      74    180    640    6    1 
dentry             34511  36820    192   20    1
#6 lore raw
In reply to Sukadev Bhattiprolu

fs/proc/base.c static void proc_flush_task_mnt(struct v · line 2583

Quoted 9 lines · Sukadev Bhattiprolu
fs/proc/base.c +1 -2
@@ -2580,8 +2580,7 @@ static void proc_flush_task_mnt(struct v
 	name.len = snprintf(buf, sizeof(buf), "%d", pid);
 	dentry = d_hash_and_lookup(mnt->mnt_root, &name);
 	if (dentry) {
-		if (!(current->flags & PF_EXITING))
-			shrink_dcache_parent(dentry);
+		shrink_dcache_parent(dentry);

What to do with original deadlock?