Re: [PATCH] Make /proc/net a symlink on /proc/self/net

Previous thread: [PATCH] Memcontrol: remove a stray ` sign from comment. by Pavel Emelyanov on Wednesday, March 5, 2008 - 4:38 am. (2 messages)

Next thread: [git patches] net driver fixes by Jeff Garzik on Wednesday, March 5, 2008 - 5:39 am. (2 messages)
From: Pavel Emelyanov
Date: Wednesday, March 5, 2008 - 5:20 am

Current /proc/net is done with so called "shadows", but current
implementation is broken and has little chances to get fixed.

The problem is that dentries subtree of /proc/net directory has
fancy revalidation rules to make processes living in different
net namespaces see different entries in /proc/net subtree, but
currently, tasks see in the /proc/net subdir the contents of any
other namespace, depending on who opened the file first.

The proposed fix is to turn /proc/net into a symlink, which points
to /proc/self/net, which in turn shows what previously was in
/proc/net - the network-related info, from the net namespace the
appropriate task lives in.

# ls -l /proc/net
lrwxrwxrwx  1 root root 8 Mar  5 15:17 /proc/net -> self/net

In other words - this behaves like /proc/mounts, but unlike
"mounts", "net" is not a file, but a directory.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 96ee899..cc43cf0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2274,6 +2274,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 	DIR("task",       S_IRUGO|S_IXUGO, task),
 	DIR("fd",         S_IRUSR|S_IXUSR, fd),
 	DIR("fdinfo",     S_IRUSR|S_IXUSR, fdinfo),
+	DIR("net",        S_IRUGO|S_IXUSR, net),
 	REG("environ",    S_IRUSR, environ),
 	INF("auxv",       S_IRUSR, pid_auxv),
 	ONE("status",     S_IRUGO, pid_status),
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 68971e6..a36ad3c 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -377,15 +377,14 @@ static struct dentry_operations proc_dentry_operations =
  * Don't create negative dentries here, return -ENOENT by hand
  * instead.
  */
-struct dentry *proc_lookup(struct inode * dir, struct dentry *dentry, struct nameidata *nd)
+struct dentry *proc_lookup_de(struct proc_dir_entry *de, struct inode *dir,
+		struct dentry *dentry)
 {
 	struct inode *inode = NULL;
-	struct proc_dir_entry * de;
 	int error = -ENOENT;
 
 	lock_kernel();
 ...
From: Paul E. McKenney
Date: Wednesday, March 5, 2008 - 12:15 pm

One question below for get_proc_task_net() -- I don't see how the
reference to a task struct is released.


This implies a get_task_struct(), as get_proc_task() invokes get_proc_task()
which invokes get_pid_task() which invokes get_task_struct().

--

From: David Miller
Date: Wednesday, March 5, 2008 - 2:29 pm

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Yes, it definitely seems to leak.
--

From: Eric W. Biederman
Date: Wednesday, March 5, 2008 - 4:59 pm

You aren't.  However we can easily make this:
	task = pid_task(proc_pid(dir), PIDTYPE_PID);
And we won't need to increment the count on the task_struct.

--

From: Paul E. McKenney
Date: Wednesday, March 5, 2008 - 5:22 pm

That looks like it should do the trick!  And the net_get() below
will allow you to safely export the "net" pointer out of an RCU
read-side critical section, so with that change looks good to me!

--

From: Pavel Emelyanov
Date: Thursday, March 6, 2008 - 1:25 am

Shame on me :( Thanks Paul, I'll prepare the fixed patch shortly,

--

From: Eric W. Biederman
Date: Wednesday, March 5, 2008 - 5:24 pm

Overall this looks good, thanks.


As a follow on patch this should be moved from /proc/<pid>/net
into /proc/<pid>/task/<tid>/net.

We don't have anything that currently forces network namespaces to be
the same between different tasks of the same task group (I just
looked), nor do we have a technical reason to require that.

So we should fix our infrastructure to include the companion of
/proc/self, a /proc/current (which points at the current task)
after which it should be about a two line change to move this
from the tgid to the task directory.

Eric
--

From: Pavel Emelyanov
Date: Thursday, March 6, 2008 - 1:40 am

This is right what I intended to tell you when you would propose
to tune the /proc/net link. I'm glad hearing, that you already

--

From: Denys Vlasenko
Date: Sunday, March 23, 2008 - 6:00 am

This broke tools which read /proc/net/dev. Under non-root,
they are no longer working. This is a regression.
--
vda
--

From: Denys Vlasenko
Date: Sunday, March 23, 2008 - 6:04 am

I see that this is already reported & fix is in queue.

Sorry for the noise.
--
vda
--

From: David Miller
Date: Sunday, March 23, 2008 - 6:16 am

From: Denys Vlasenko <vda.linux@googlemail.com>

This is fixed with a commit that was made yesterday.
--

Previous thread: [PATCH] Memcontrol: remove a stray ` sign from comment. by Pavel Emelyanov on Wednesday, March 5, 2008 - 4:38 am. (2 messages)

Next thread: [git patches] net driver fixes by Jeff Garzik on Wednesday, March 5, 2008 - 5:39 am. (2 messages)