All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Galbraith <efault@gmx.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Subject: [RFC PATCH] namespaces: fix leak on fork() failure
Date: Sat, 28 Apr 2012 11:19:50 +0200	[thread overview]
Message-ID: <1335604790.5995.22.camel@marge.simpson.net> (raw)

[-- Attachment #1: Type: text/plain, Size: 2448 bytes --]

Greetings,

The attached testcase induces quite a bit of pid/mnt namespace leakage.
The below fixes up one of these leaks.  There's still at least one pid
namespace leak left, that being the final put_pid() in softirq context
goes missing.

A trace of the leak that's left shows... 
vsftpd-5055  [003] ....  3921.490806: proc_set_super: get_pid_ns: 0xffff8801c996e988 count:1->2
vsftpd-5055  [003] ....  3921.490823: alloc_pid: get_pid_ns: 0xffff8801c996e988 count:2->3
vsftpd-5102  [003] ....  3921.502565: switch_task_namespaces: exiting: 0xffff8801c996e988 count:3
vsftpd-5102  [003] ....  3921.522296: free_nsproxy: put_pid_ns: 0xffff8801c996e988 count:3->2
vsftpd-5055  [003] ....  3921.574201: proc_kill_sb: put_pid_ns: 0xffff8801c996e988 count:2->1

..but that should be..

vsftpd-5055  [003] ....  3921.497313: proc_set_super: get_pid_ns: 0xffff8801c6e65ff0 count:1->2
vsftpd-5055  [003] ....  3921.497330: alloc_pid: get_pid_ns: 0xffff8801c6e65ff0 count:2->3
vsftpd-5124  [003] ....  3921.502977: switch_task_namespaces: exiting: 0xffff8801c6e65ff0 count:3
vsftpd-5124  [003] ....  3921.522308: free_nsproxy: put_pid_ns: 0xffff8801c6e65ff0 count:3->2
vsftpd-5055  [003] ....  3921.698349: proc_kill_sb: put_pid_ns: 0xffff8801c6e65ff0 count:2->1
ksoftirqd/3-16    [003] ..s.  3921.702182: put_pid: put_pid_ns: 0xffff8801c6e65ff0 count:1->0

Anyway, here's what I did for one of the little buggers.

SIGCHLD delivery during fork() may cause failure, resulting in the aborted
child being cloned with CLONE_NEWPID leaking namespaces due to proc being
mounted during pid namespace creation, but not unmounted on fork() failure.

Call pid_ns_release_proc() to prevent the leaks.

Signed-off-by: Mike Galbraith <efault@gmx.de>
 
 kernel/nsproxy.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index b576f7f..fd751d3 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -216,6 +216,14 @@ void switch_task_namespaces(struct task_struct *p, struct nsproxy *new)
 	rcu_assign_pointer(p->nsproxy, new);
 
 	if (ns && atomic_dec_and_test(&ns->count)) {
+		/* Handle fork() failure, unmount proc before proceeding */
+		if (unlikely(!new && !((p->flags & PF_EXITING)))) {
+			struct pid_namespace *pid_ns = ns->pid_ns;
+
+			if (pid_ns && pid_ns != &init_pid_ns)
+				pid_ns_release_proc(pid_ns);
+		}
+
 		/*
 		 * wait for others to get what they want from this nsproxy.
 		 *


[-- Attachment #2: vsftpd.c --]
[-- Type: text/x-csrc, Size: 2181 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <sched.h>
#include <linux/sched.h>
#include <unistd.h>
#include <sys/syscall.h>

#include <sys/types.h>
#include <sys/wait.h>
#include <signal.h>
#include <string.h>

#if !defined(WITH_SIGCHLD)
#define WITH_SIGCHLD 1
#endif

#if WITH_SIGCHLD == 1
/*
 * vsftpd 
 * sysutil.c vsf_sysutil_wait_reap_one()
 * standalone.c handle_sigchld()
 * 
 */
int vsf_sysutil_wait_reap_one(void)
{
    int retval = waitpid(-1, NULL, WNOHANG);
    if (retval == 0 || (retval < 0 && errno == ECHILD)) {
        /* No more children */
        return 0;
    }
    if (retval < 0) {
        perror("waitpid");
        exit(EXIT_FAILURE);
    }
    /* Got one */
    return retval;
}

int received;
int reaped;

void handle_sigchld(int sig)
{
    unsigned int reap_one = 1;

    received++;
    while (reap_one) {
        reap_one = (unsigned int) vsf_sysutil_wait_reap_one();
	if (reap_one)
            reaped++;
    }
}
#endif

int zombies;

int main(int argc, char *argv[])
{
    int i, ret;

#if WITH_SIGCHLD == 1
    /*
     * vsftpd sysutil.c vsf_sysutil_set_sighandler()
     */
    struct sigaction sa;
    memset(&sa, 0, sizeof(sa));
    sa.sa_handler = handle_sigchld;
    if (-1 == sigfillset(&sa.sa_mask)) {
        perror("sigfillset");
        exit(EXIT_FAILURE);
    }
    if (-1 == sigaction(SIGCHLD, &sa, NULL)) {
        perror("sigaction");
        exit(EXIT_FAILURE);
    }
    fprintf(stderr, "SIGCHLD handler enabled\n");
#else
    fprintf(stderr, "SIGCHLD handler not enabled\n");
#endif

    for (i = 0; i < 100; i++) {

//        if (0 == (ret = syscall(__NR_clone, CLONE_NEWPID | CLONE_NEWIPC | CLONE_NEWNET | CLONE_NEWUSER | SIGCHLD, NULL)))
        if (0 == (ret = syscall(__NR_clone, CLONE_NEWPID | SIGCHLD, NULL)))
            return 0;

        if (-1 == ret) {
            perror("clone");
            exit(EXIT_FAILURE);
        }

    }
#if 1
    while (1) {
	int res = waitpid(-1, NULL, WNOHANG);
	if (res < 0)
		break;
	if (!res)
		continue;
        zombies++;
    }
//    printf("received %d signals, reaped %d - %d zombies left\n", received, reaped, zombies);
//    sleep(1);
#endif
    return 0;
}

             reply	other threads:[~2012-04-28  9:19 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-28  9:19 Mike Galbraith [this message]
2012-04-28 14:26 ` [RFC PATCH] namespaces: fix leak on fork() failure Oleg Nesterov
2012-04-29  4:13   ` Mike Galbraith
2012-04-29  7:57   ` Eric W. Biederman
2012-04-29  9:49     ` Mike Galbraith
2012-04-29 16:58     ` Oleg Nesterov
2012-04-30  2:59       ` Eric W. Biederman
2012-04-30  3:25         ` Mike Galbraith
2012-05-02 12:40         ` Oleg Nesterov
2012-05-02 17:37           ` Eric W. Biederman
2012-04-30  3:01       ` [PATCH] " Mike Galbraith
     [not found]         ` <m1zk9rmyh4.fsf@fess.ebiederm.org>
2012-05-01 20:42           ` Andrew Morton
2012-05-03  3:12             ` Mike Galbraith
2012-05-03 14:56               ` Mike Galbraith
2012-05-04  4:27                 ` Mike Galbraith
2012-05-04  7:55                   ` Eric W. Biederman
2012-05-04  8:34                     ` Mike Galbraith
2012-05-04  9:45                     ` Mike Galbraith
2012-05-04 14:13                       ` Eric W. Biederman
2012-05-04 14:49                         ` Mike Galbraith
2012-05-04 15:36                           ` Eric W. Biederman
2012-05-04 16:57                             ` Mike Galbraith
2012-05-04 20:29                               ` Eric W. Biederman
2012-05-05  5:56                                 ` Mike Galbraith
2012-05-05  6:08                                   ` Mike Galbraith
2012-05-05  7:12                                     ` Mike Galbraith
2012-05-05 11:37                                       ` Eric W. Biederman
2012-05-07 21:51                                       ` [PATCH] vfs: Speed up deactivate_super for non-modular filesystems Eric W. Biederman
2012-05-07 22:17                                         ` Al Viro
2012-05-07 23:56                                           ` Paul E. McKenney
2012-05-08  1:07                                             ` Eric W. Biederman
2012-05-08  4:53                                               ` Mike Galbraith
2012-05-09  7:55                                               ` Nick Piggin
2012-05-09 11:02                                                 ` Eric W. Biederman
2012-05-09 11:02                                                   ` Eric W. Biederman
2012-05-15  8:40                                                   ` Nick Piggin
2012-05-16  0:34                                                     ` Eric W. Biederman
2012-05-16  0:34                                                       ` Eric W. Biederman
2012-05-09 13:59                                                 ` Paul E. McKenney
2012-05-04  8:03                 ` [PATCH] Re: [RFC PATCH] namespaces: fix leak on fork() failure Eric W. Biederman
2012-05-04  8:19                   ` Mike Galbraith
2012-05-04  8:54                     ` Mike Galbraith
2012-05-07  0:32             ` [PATCH 0/3] pidns: Closing the pid namespace exit race Eric W. Biederman
2012-05-07  0:33               ` [PATCH 1/3] pidns: Use task_active_pid_ns in do_notify_parent Eric W. Biederman
2012-05-07  0:35               ` [PATCH 2/3] pidns: Guarantee that the pidns init will be the last pidns process reaped Eric W. Biederman
2012-05-08 22:50                 ` Andrew Morton
2012-05-16 18:39                 ` Oleg Nesterov
2012-05-16 19:34                   ` Oleg Nesterov
2012-05-16 20:54                   ` Eric W. Biederman
2012-05-17 17:00                     ` Oleg Nesterov
2012-05-17 21:46                       ` Eric W. Biederman
2012-05-18 12:39                         ` Oleg Nesterov
2012-05-19  0:03                           ` Eric W. Biederman
2012-05-21 12:44                             ` Oleg Nesterov
2012-05-22  0:16                               ` Eric W. Biederman
2012-05-22  0:20                               ` [PATCH] pidns: Guarantee that the pidns init will be the last pidns process reaped. v2 Eric W. Biederman
2012-05-22 16:54                                 ` Oleg Nesterov
2012-05-22 19:23                                 ` Andrew Morton
2012-05-23 14:52                                   ` Oleg Nesterov
2012-05-25 15:15                                     ` [PATCH -mm] pidns-guarantee-that-the-pidns-init-will-be-the-last-pidns-process-r eaped-v2-fix-fix Oleg Nesterov
2012-05-25 15:59                                       ` [PATCH -mm 0/1] pidns: find_new_reaper() can no longer switch to init_pid_ns.child_reaper Oleg Nesterov
2012-05-25 16:00                                         ` [PATCH -mm 1/1] " Oleg Nesterov
2012-05-25 21:43                                           ` Eric W. Biederman
2012-05-27 19:10                                             ` [PATCH v2 -mm 0/1] " Oleg Nesterov
2012-05-27 19:11                                               ` [PATCH v2 -mm 1/1] " Oleg Nesterov
2012-05-29  6:34                                                 ` Eric W. Biederman
2012-05-25 21:25                                       ` [PATCH -mm] pidns-guarantee-that-the-pidns-init-will-be-the-last-pidns-process-r eaped-v2-fix-fix Eric W. Biederman
2012-05-27 18:41                                         ` [PATCH -mm v2] " Oleg Nesterov
2012-05-07  0:35               ` [PATCH 3/3] pidns: Make killed children autoreap Eric W. Biederman
2012-05-08 22:51                 ` Andrew Morton
2012-04-30 13:57 ` [RFC PATCH] namespaces: fix leak on fork() failure Mike Galbraith

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1335604790.5995.22.camel@marge.simpson.net \
    --to=efault@gmx.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.