public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, andi@firstfloor.org,
	David Howells <dhowells@redhat.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	Roland McGrath <roland@redhat.com>
Subject: [PATCH 2/4] coredump: cleanup "ispipe" code
Date: Tue, 16 Mar 2010 20:38:53 +0100	[thread overview]
Message-ID: <20100316193853.GC31632@redhat.com> (raw)
In-Reply-To: <20100316193750.GA31632@redhat.com>

- kill "int dump_count", argv_split(argcp) accepts argcp == NULL.

- move "int dump_count" under " if (ispipe)" branch, fail_dropcount
  can check ispipe.

- move "char **helper_argv" as well, change the code to do argv_free()
  right after call_usermodehelper_fns().

- If call_usermodehelper_fns() fails goto close_fail label instead
  of closing the file by hand.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 fs/exec.c |   39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

--- 34-rc1/fs/exec.c~2_IFIFO	2010-03-16 18:06:12.000000000 +0100
+++ 34-rc1/fs/exec.c	2010-03-16 18:09:13.000000000 +0100
@@ -1838,10 +1838,7 @@ void do_coredump(long signr, int exit_co
 	struct cred *cred;
 	int retval = 0;
 	int flag = 0;
-	int ispipe = 0;
-	char **helper_argv = NULL;
-	int helper_argc = 0;
-	int dump_count = 0;
+	int ispipe;
 	static atomic_t core_dump_count = ATOMIC_INIT(0);
 	struct coredump_params cprm = {
 		.signr = signr,
@@ -1911,6 +1908,9 @@ void do_coredump(long signr, int exit_co
 	unlock_kernel();
 
  	if (ispipe) {
+		int dump_count;
+		char **helper_argv;
+
 		if (cprm.limit == 1) {
 			/*
 			 * Normally core limits are irrelevant to pipes, since
@@ -1932,6 +1932,7 @@ void do_coredump(long signr, int exit_co
 			printk(KERN_WARNING "Aborting core\n");
 			goto fail_unlock;
 		}
+		cprm.limit = RLIM_INFINITY;
 
 		dump_count = atomic_inc_return(&core_dump_count);
 		if (core_pipe_limit && (core_pipe_limit < dump_count)) {
@@ -1941,26 +1942,21 @@ void do_coredump(long signr, int exit_co
 			goto fail_dropcount;
 		}
 
-		helper_argv = argv_split(GFP_KERNEL, corename+1, &helper_argc);
+		helper_argv = argv_split(GFP_KERNEL, corename+1, NULL);
 		if (!helper_argv) {
 			printk(KERN_WARNING "%s failed to allocate memory\n",
 			       __func__);
 			goto fail_dropcount;
 		}
 
-		cprm.limit = RLIM_INFINITY;
-
-		/* SIGPIPE can happen, but it's just never processed */
-		cprm.file = NULL;
-		if (call_usermodehelper_fns(helper_argv[0], helper_argv, NULL,
-					    UMH_WAIT_EXEC, umh_pipe_setup,
-					    NULL, &cprm)) {
-			if (cprm.file)
-				filp_close(cprm.file, NULL);
-
+		retval = call_usermodehelper_fns(helper_argv[0], helper_argv,
+					NULL, UMH_WAIT_EXEC, umh_pipe_setup,
+					NULL, &cprm);
+		argv_free(helper_argv);
+		if (retval) {
  			printk(KERN_INFO "Core dump to %s pipe failed\n",
 			       corename);
-			goto fail_dropcount;
+			goto close_fail;
  		}
 	} else {
 		struct inode *inode;
@@ -2000,17 +1996,16 @@ void do_coredump(long signr, int exit_co
 	retval = binfmt->core_dump(&cprm);
 	if (retval)
 		current->signal->group_exit_code |= 0x80;
-close_fail:
+
 	if (ispipe && core_pipe_limit)
 		wait_for_dump_helpers(cprm.file);
-	filp_close(cprm.file, NULL);
+close_fail:
+	if (cprm.file)
+		filp_close(cprm.file, NULL);
 fail_dropcount:
-	if (dump_count)
+	if (ispipe)
 		atomic_dec(&core_dump_count);
 fail_unlock:
-	if (helper_argv)
-		argv_free(helper_argv);
-
 	revert_creds(old_cred);
 	put_cred(cred);
 	coredump_finish(mm);


  parent reply	other threads:[~2010-03-16 19:40 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-15 12:29 [PATCH 0/2] exec: refactor how call_usermodehelper works, and update the sense of the core_pipe recursion check (v4 rediff) Neil Horman
2010-03-15 12:33 ` [PATCH 1/2] kmod: add init function to usermodehelper Neil Horman
2010-03-15 17:34   ` Oleg Nesterov
2010-03-15 17:56     ` Neil Horman
2010-03-15 12:36 ` [PATCH 2/2] exec: replace call_usermodehelper_pipe with use of umh init function and resolve limit Neil Horman
2010-03-15 17:39   ` Oleg Nesterov
2010-03-15 19:46 ` [PATCH 0/6] umh: keys, signals, misc Oleg Nesterov
2010-03-15 19:46   ` [PATCH 1/6] umh: creds: convert call_usermodehelper_keys() to use subprocess_info->init() Oleg Nesterov
2010-03-15 19:47   ` [PATCH 2/6] umh: creds: kill subprocess_info->cred logic Oleg Nesterov
2010-03-15 19:47   ` [PATCH 3/6] call_usermodehelper: no need to unblock signals Oleg Nesterov
2010-03-15 19:48   ` [PATCH 4/6] wait_for_helper: SIGCHLD from user-space can lead to use-after-free Oleg Nesterov
2010-03-15 19:48   ` [PATCH 5/6] call_usermodehelper: simplify/fix UMH_NO_WAIT case Oleg Nesterov
2010-03-15 19:49   ` [PATCH 6/6] call_usermodehelper: UMH_WAIT_EXEC ignores kernel_thread() failure Oleg Nesterov
2010-03-16 19:37   ` [PATCH 0/4] do_coredump: cleanups Oleg Nesterov
2010-03-16 19:38     ` [PATCH 1/4] coredump: factor out the not-ispipe file checks Oleg Nesterov
2010-03-16 19:38     ` Oleg Nesterov [this message]
2010-03-16 19:39     ` [PATCH 3/4] coredump: factor out put_cred() calls Oleg Nesterov
2010-03-16 19:39     ` [PATCH 4/4] coredump: shift down_write(mmap_sem) into coredump_wait() Oleg Nesterov

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=20100316193853.GC31632@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=roland@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox