On Wed, Oct 10, 2012 at 05:40:22PM +0200, Oleg Nesterov wrote:
> On 10/09, Eric W. Biederman wrote:
> >
> > Oleg Nesterov <[email protected]> writes:
> >
> > >> Someone is abusing something here, please don't change the kref api to
> > >> work around someone using it improperly.
> > >>
> > >> So no, please don't apply this at all.
> > >
> > > Because this is wrong or because not kosher?
> >
> > Oleg it is easy enough to not use the unnecesary kref abstraction.
> >
> > We can directly use atomic_dec_and_test.  Since the pid namespace no
> > longer fits into the narrow confines of the kref abstraction there is
> > no need to use it.
> 
> Ah OK.
> 
> I misunderstood Greg as if he nacked the attempt to dec kref->refcount
> without kref_put(release).
> 
> I agree we can do this without the new helper. And anyway Cyrill sent v2.

Oleg, mind to take a look on this one slightly updated?
---
From: Cyrill Gorcunov <[email protected]>
Subject: [PATCH] pidns: remove recursion from free_pid_ns() v5

The free_pid_ns function done in recursion fashion:

free_pid_ns(parent)
  put_pid_ns(parent)
    kref_put(&ns->kref, free_pid_ns);
      free_pid_ns

thus if there was a huge nesting of namespaces the userspace
may trigger avalanche calling of free_pid_ns leading to
kernel stack exhausting and a panic eventually.

This patch turns the recursion into iterative loop.

v5:
 - Drop @ret variable (from oleg@)

Based-on-patch-by: Andrew Vagin <[email protected]>
Signed-off-by: Cyrill Gorcunov <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Greg KH <[email protected]>
Cc: <[email protected]>
---
 include/linux/pid_namespace.h |   10 ++++++++--
 kernel/pid_namespace.c        |    7 +------
 2 files changed, 9 insertions(+), 8 deletions(-)

Index: linux-2.6.git/include/linux/pid_namespace.h
===================================================================
--- linux-2.6.git.orig/include/linux/pid_namespace.h
+++ linux-2.6.git/include/linux/pid_namespace.h
@@ -53,8 +53,14 @@ extern int reboot_pid_ns(struct pid_name
 
 static inline void put_pid_ns(struct pid_namespace *ns)
 {
-       if (ns != &init_pid_ns)
-               kref_put(&ns->kref, free_pid_ns);
+       struct pid_namespace *parent;
+
+       while (ns != &init_pid_ns) {
+               parent = ns->parent;
+               if (!kref_put(&ns->kref, free_pid_ns))
+                       break;
+               ns = parent;
+       }
 }
 
 #else /* !CONFIG_PID_NS */
Index: linux-2.6.git/kernel/pid_namespace.c
===================================================================
--- linux-2.6.git.orig/kernel/pid_namespace.c
+++ linux-2.6.git/kernel/pid_namespace.c
@@ -134,15 +134,10 @@ struct pid_namespace *copy_pid_ns(unsign
 
 void free_pid_ns(struct kref *kref)
 {
-       struct pid_namespace *ns, *parent;
+       struct pid_namespace *ns;
 
        ns = container_of(kref, struct pid_namespace, kref);
-
-       parent = ns->parent;
        destroy_pid_namespace(ns);
-
-       if (parent != NULL)
-               put_pid_ns(parent);
 }
 
 void zap_pid_ns_processes(struct pid_namespace *pid_ns)
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to