All of lore.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 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.