public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash
@ 2012-02-03 15:44 Heiko Carstens
  2012-02-03 16:04 ` Oleg Nesterov
  0 siblings, 1 reply; 6+ messages in thread
From: Heiko Carstens @ 2012-02-03 15:44 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Oleg Nesterov, Paul Menage
  Cc: linux-kernel, Sebastian Ott

Hi all,

Sebastian today sent me a dump with this crash:

[    9.642907] Unable to handle kernel pointer dereference at virtual kernel address 0000000039768000
[    9.642918] Oops: 0011 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[    9.642934] Modules linked in: qeth_l3 lcs ctcm fsm vmur qeth ccwgroup autofs4 [last unloaded: scsi_wait_scan]
[    9.642965] CPU: 0 Not tainted 3.3.0-rc2-00037-gbd3ce7d-dirty #48
[    9.643011] Process kworker/u:3 (pid: 245, task: 000000003a3dc840, ksp: 0000000039453818)
[    9.643015] Krnl PSW : 0704000180000000 0000000000282e94 (setup_new_exec+0xa0/0x374)
[    9.643026]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[    9.643032] Krnl GPRS: 0000000030844dcb fffffffffffffffd 0000000039768000 0000000000000000
[    9.643039]            00000000000000cd 0000000000243c18 00000000000001f8 0000000039453dd0
[    9.643045]            000000003dc55220 0000000000000000 00000000006f4800 000000003dc55220
[    9.643051]            0000000000000000 00000000005dd958 00000000002830f0 0000000039453a18
[    9.643067] Krnl Code: 0000000000282e84: c0e5ffffff52        brasl   %r14,282d28
[    9.643079]            0000000000282e8a: e320b0c80004        lg      %r2,200(%r11)
[    9.643092]           #0000000000282e90: a7380000            lhi     %r3,0
[    9.643108]           >0000000000282e94: e31020000090        llgc    %r1,0(%r2)
[    9.643123]            0000000000282e9a: 41202001            la      %r2,1(%r2)
[    9.643162]            0000000000282e9e: 1241                ltr     %r4,%r1
[    9.643168]            0000000000282ea0: a7840018            brc     8,282ed0
[    9.643175]            0000000000282ea4: a74e002f            chi     %r4,47
[    9.643183] Call Trace:
[    9.643186] ([<0000000000282e2c>] setup_new_exec+0x38/0x374)
[    9.643192]  [<00000000002dd12e>] load_elf_binary+0x402/0x1bf4
[    9.643201]  [<0000000000280a42>] search_binary_handler+0x38e/0x5bc
[    9.643210]  [<0000000000282b6c>] do_execve_common+0x410/0x514
[    9.643218]  [<0000000000282cb6>] do_execve+0x46/0x58
[    9.643225]  [<00000000005bce58>] kernel_execve+0x28/0x70
[    9.643236]  [<000000000014ba2e>] ____call_usermodehelper+0x102/0x140
[    9.643245]  [<00000000005bc8da>] kernel_thread_starter+0x6/0xc
[    9.643254]  [<00000000005bc8d4>] kernel_thread_starter+0x0/0xc
[    9.643264] INFO: lockdep is turned off.
[    9.643269] Last Breaking-Event-Address:
[    9.643275]  [<00000000002830f0>] setup_new_exec+0x2fc/0x374
[    9.643311]  
[    9.643315] Kernel panic - not syncing: Fatal exception: panic_on_oops

As it happens it is a use-after-free bug. It crashes in setup_new_exec() when
trying to dereference name:

setup_new_exec(...)
	[...]
	name = bprm->filename;

	/* Copies the binary name from after last slash */
	for (i=0; (ch = *(name++)) != '\0';) {	<-- crashes here
		if (ch == '/')

Looking into the dump I was able to tell that the piece of memory got freed
by cgroup_release_agent().
Which has the following code sequence:

static void cgroup_release_agent(struct work_struct *work)
{
		[...]
		agentbuf = kstrdup(cgrp->root->release_agent_path, GFP_KERNEL);
		[...]
		i = 0;
		argv[i++] = agentbuf;
		[...]
		call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
		[...]
		kfree(agentbuf);
		[...]
}

So obviously cgroup_release_agent() freed the filename before do_execve()
was finished.

call_usermodehelper() will enqueue a struct work which will call
__call_usermodehelper() which in turn will create a kernel_thread which
executes ____call_usermodehelper(). Here it sets the flag CLONE_VFORK to
ensure that all needed structures stay alive until the code from
____call_usermodehelper() gets replaced by the to be executed process.

So due to CLONE_VFORK kernel_thread() will block until the child process
has replaced it's mm, which happens in
load_elf_binary() -> flush_old_exec() -> exec_mmap() -> mm_release()
and which subsequently wakes up the parent again. So the parent will
continue and in the end return from call_usermodehelper() and free
the passed path (aka agentbuf).

However the call to flush_old_exec() happens right _before_ the call
to setup_new_exec() which still needs the path and may crash if
it got freed (like it did this time).

So the question is: what is broken? The cgroup stuff which doesn't take
into account that the passed path may still be in use and hence can't
be freed (simple fix would be to simply use UMH_WAIT_PROC instead).
Or is it that call_usermodehelper() still uses the passed path after
it returned?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash
  2012-02-03 15:44 cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash Heiko Carstens
@ 2012-02-03 16:04 ` Oleg Nesterov
  2012-02-03 16:48   ` Linus Torvalds
  0 siblings, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2012-02-03 16:04 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Linus Torvalds, Andrew Morton, Paul Menage, linux-kernel,
	Sebastian Ott

On 02/03, Heiko Carstens wrote:
>
> setup_new_exec(...)
> 	[...]
> 	name = bprm->filename;
>
> 	/* Copies the binary name from after last slash */
> 	for (i=0; (ch = *(name++)) != '\0';) {	<-- crashes here
> 		if (ch == '/')

Ough, and this happens after flush_old_exec()...

> Looking into the dump I was able to tell that the piece of memory got freed
> by cgroup_release_agent().
> Which has the following code sequence:
>
> static void cgroup_release_agent(struct work_struct *work)
> {
> 		[...]
> 		agentbuf = kstrdup(cgrp->root->release_agent_path, GFP_KERNEL);
> 		[...]
> 		i = 0;
> 		argv[i++] = agentbuf;
> 		[...]
> 		call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
> 		[...]
> 		kfree(agentbuf);
> 		[...]
> }
>
> So obviously cgroup_release_agent() freed the filename before do_execve()
> was finished.

Good catch.

> So the question is: what is broken? The cgroup stuff which doesn't take
> into account that the passed path may still be in use and hence can't
> be freed (simple fix would be to simply use UMH_WAIT_PROC instead).
> Or is it that call_usermodehelper() still uses the passed path after
> it returned?

Well, it seems that do_coredump() has the same problem.

Can't we simply move that code into flush_old_exec() ? (wrapped into
the new helper).

Oleg.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash
  2012-02-03 16:04 ` Oleg Nesterov
@ 2012-02-03 16:48   ` Linus Torvalds
  2012-02-04 10:03     ` Heiko Carstens
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2012-02-03 16:48 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Heiko Carstens, Andrew Morton, Paul Menage, linux-kernel,
	Sebastian Ott

On Fri, Feb 3, 2012 at 8:04 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>
> Can't we simply move that code into flush_old_exec() ? (wrapped into
> the new helper).

Sure. It would kind of make sense to do it as part of exec_mmap().
That's what associates us with the new mm, after all.

That said, I think my *preferred* approach would be to still do the final

    set_task_comm(current, tcomm);

in setup_new_exec(), because that's really when we set up the new mm.

So my preferred solution would be to simply move the "char tcomm[];"
array from the stack (currently automatic in setup_new_exec()) into
the struct linux_binprm, and then copy it from the filename early. We
could copy it arbitrarily early, perhaps in "prepare_binprm()".

Hmm?

                       Linus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash
  2012-02-03 16:48   ` Linus Torvalds
@ 2012-02-04 10:03     ` Heiko Carstens
  2012-02-04 15:16       ` Linus Torvalds
  0 siblings, 1 reply; 6+ messages in thread
From: Heiko Carstens @ 2012-02-04 10:03 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Oleg Nesterov, Andrew Morton, Paul Menage, linux-kernel,
	Sebastian Ott

On Fri, Feb 03, 2012 at 08:48:08AM -0800, Linus Torvalds wrote:
> On Fri, Feb 3, 2012 at 8:04 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > Can't we simply move that code into flush_old_exec() ? (wrapped into
> > the new helper).
> 
> Sure. It would kind of make sense to do it as part of exec_mmap().
> That's what associates us with the new mm, after all.
> 
> That said, I think my *preferred* approach would be to still do the final
> 
>     set_task_comm(current, tcomm);
> 
> in setup_new_exec(), because that's really when we set up the new mm.
> 
> So my preferred solution would be to simply move the "char tcomm[];"
> array from the stack (currently automatic in setup_new_exec()) into
> the struct linux_binprm, and then copy it from the filename early. We
> could copy it arbitrarily early, perhaps in "prepare_binprm()".
> 
> Hmm?

Something like the patch below? Still boots...

>From e117282bdd4d563dd3c9c4e1d059550657834a55 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Sat, 4 Feb 2012 10:47:10 +0100
Subject: [PATCH] exec: fix use-after-free bug in setup_new_exec()

Setting the task name is done within setup_new_exec() by accessing
bprm->filename. However this happens after flush_old_exec().
This may result in a use after free bug, flush_old_exec() may
"complete" vfork_done, which will wake up the parent which in turn
may free the passed in filename.
To fix this add a new tcomm field in struct linux_binprm which
contains the now early generated task name until it is used.

Fixes this bug on s390:

[    9.642907] Unable to handle kernel pointer dereference at virtual kernel address 0000000039768000
[    9.642918] Oops: 0011 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[    9.642934] Modules linked in: qeth_l3 lcs ctcm fsm vmur qeth ccwgroup autofs4 [last unloaded: scsi_wait_scan]
[    9.642965] CPU: 0 Not tainted 3.3.0-rc2-00037-gbd3ce7d-dirty #48
[    9.643011] Process kworker/u:3 (pid: 245, task: 000000003a3dc840, ksp: 0000000039453818)
[    9.643015] Krnl PSW : 0704000180000000 0000000000282e94 (setup_new_exec+0xa0/0x374)
[    9.643026]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[    9.643032] Krnl GPRS: 0000000030844dcb fffffffffffffffd 0000000039768000 0000000000000000
[    9.643039]            00000000000000cd 0000000000243c18 00000000000001f8 0000000039453dd0
[    9.643045]            000000003dc55220 0000000000000000 00000000006f4800 000000003dc55220
[    9.643051]            0000000000000000 00000000005dd958 00000000002830f0 0000000039453a18
[    9.643067] Krnl Code: 0000000000282e84: c0e5ffffff52        brasl   %r14,282d28
[    9.643079]            0000000000282e8a: e320b0c80004        lg      %r2,200(%r11)
[    9.643092]           #0000000000282e90: a7380000            lhi     %r3,0
[    9.643108]           >0000000000282e94: e31020000090        llgc    %r1,0(%r2)
[    9.643123]            0000000000282e9a: 41202001            la      %r2,1(%r2)
[    9.643162]            0000000000282e9e: 1241                ltr     %r4,%r1
[    9.643168]            0000000000282ea0: a7840018            brc     8,282ed0
[    9.643175]            0000000000282ea4: a74e002f            chi     %r4,47
[    9.643183] Call Trace:
[    9.643186] ([<0000000000282e2c>] setup_new_exec+0x38/0x374)
[    9.643192]  [<00000000002dd12e>] load_elf_binary+0x402/0x1bf4
[    9.643201]  [<0000000000280a42>] search_binary_handler+0x38e/0x5bc
[    9.643210]  [<0000000000282b6c>] do_execve_common+0x410/0x514
[    9.643218]  [<0000000000282cb6>] do_execve+0x46/0x58
[    9.643225]  [<00000000005bce58>] kernel_execve+0x28/0x70
[    9.643236]  [<000000000014ba2e>] ____call_usermodehelper+0x102/0x140
[    9.643245]  [<00000000005bc8da>] kernel_thread_starter+0x6/0xc
[    9.643254]  [<00000000005bc8d4>] kernel_thread_starter+0x0/0xc
[    9.643264] INFO: lockdep is turned off.
[    9.643269] Last Breaking-Event-Address:
[    9.643275]  [<00000000002830f0>] setup_new_exec+0x2fc/0x374
[    9.643311]
[    9.643315] Kernel panic - not syncing: Fatal exception: panic_on_oops

Reported-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 fs/exec.c               |   32 +++++++++++++++-----------------
 include/linux/binfmts.h |    3 ++-
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index aeb135c..0ad32e0 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1094,6 +1094,7 @@ int flush_old_exec(struct linux_binprm * bprm)
 		goto out;
 
 	bprm->mm = NULL;		/* We're using it now */
+	bprm->filename = NULL;
 
 	set_fs(USER_DS);
 	current->flags &= ~(PF_RANDOMIZE | PF_KTHREAD);
@@ -1116,10 +1117,6 @@ EXPORT_SYMBOL(would_dump);
 
 void setup_new_exec(struct linux_binprm * bprm)
 {
-	int i, ch;
-	const char *name;
-	char tcomm[sizeof(current->comm)];
-
 	arch_pick_mmap_layout(current->mm);
 
 	/* This is the point of no return */
@@ -1130,18 +1127,7 @@ void setup_new_exec(struct linux_binprm * bprm)
 	else
 		set_dumpable(current->mm, suid_dumpable);
 
-	name = bprm->filename;
-
-	/* Copies the binary name from after last slash */
-	for (i=0; (ch = *(name++)) != '\0';) {
-		if (ch == '/')
-			i = 0; /* overwrite what we wrote */
-		else
-			if (i < (sizeof(tcomm) - 1))
-				tcomm[i++] = ch;
-	}
-	tcomm[i] = '\0';
-	set_task_comm(current, tcomm);
+	set_task_comm(current, bprm->tcomm);
 
 	/* Set the new mm task size. We have to do that late because it may
 	 * depend on TIF_32BIT which is only updated in flush_thread() on
@@ -1275,7 +1261,8 @@ int prepare_binprm(struct linux_binprm *bprm)
 {
 	umode_t mode;
 	struct inode * inode = bprm->file->f_path.dentry->d_inode;
-	int retval;
+	int i, ch, retval;
+	const char *name;
 
 	mode = inode->i_mode;
 	if (bprm->file->f_op == NULL)
@@ -1310,6 +1297,17 @@ int prepare_binprm(struct linux_binprm *bprm)
 		return retval;
 	bprm->cred_prepared = 1;
 
+	/* Copies the binary name from after last slash */
+	name = bprm->filename;
+	for (i = 0; (ch = *(name++)) != '\0';) {
+		if (ch == '/')
+			i = 0; /* overwrite what we wrote */
+		else
+			if (i < (sizeof(bprm->tcomm) - 1))
+				bprm->tcomm[i++] = ch;
+	}
+	bprm->tcomm[i] = '\0';
+
 	memset(bprm->buf, 0, BINPRM_BUF_SIZE);
 	return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
 }
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index fd88a39..0092102 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -18,7 +18,7 @@ struct pt_regs;
 #define BINPRM_BUF_SIZE 128
 
 #ifdef __KERNEL__
-#include <linux/list.h>
+#include <linux/sched.h>
 
 #define CORENAME_MAX_SIZE 128
 
@@ -58,6 +58,7 @@ struct linux_binprm {
 	unsigned interp_flags;
 	unsigned interp_data;
 	unsigned long loader, exec;
+	char tcomm[TASK_COMM_LEN];
 };
 
 #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0
-- 
1.7.9


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash
  2012-02-04 10:03     ` Heiko Carstens
@ 2012-02-04 15:16       ` Linus Torvalds
  2012-02-05  4:12         ` Heiko Carstens
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2012-02-04 15:16 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Oleg Nesterov, Andrew Morton, Paul Menage, linux-kernel,
	Sebastian Ott

On Sat, Feb 4, 2012 at 2:03 AM, Heiko Carstens
<heiko.carstens@de.ibm.com> wrote:
>
> Something like the patch below? Still boots...

Yes, except that if we touch the comm copying code, let's do what Oleg
suggested and make it a helper function to create the "comm[]" data
array instead of open-coding it and adding new random local variables
to prepare_binprm().

But looks good otherwise. I'm a *bit* nervous that some
prepare_binprm() caller hasn't set up bprm->filename that early yet,
though. It seems like we expect that to be set up by the caller, which
seems a bit odd, actually.

                    Linus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash
  2012-02-04 15:16       ` Linus Torvalds
@ 2012-02-05  4:12         ` Heiko Carstens
  0 siblings, 0 replies; 6+ messages in thread
From: Heiko Carstens @ 2012-02-05  4:12 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Oleg Nesterov, Andrew Morton, Paul Menage, linux-kernel,
	Sebastian Ott

On Sat, Feb 04, 2012 at 07:16:51AM -0800, Linus Torvalds wrote:
> On Sat, Feb 4, 2012 at 2:03 AM, Heiko Carstens
> <heiko.carstens@de.ibm.com> wrote:
> >
> > Something like the patch below? Still boots...
> 
> Yes, except that if we touch the comm copying code, let's do what Oleg
> suggested and make it a helper function to create the "comm[]" data
> array instead of open-coding it and adding new random local variables
> to prepare_binprm().
> 
> But looks good otherwise. I'm a *bit* nervous that some
> prepare_binprm() caller hasn't set up bprm->filename that early yet,
> though. It seems like we expect that to be set up by the caller, which
> seems a bit odd, actually.

Ok, I moved it to flush_old_exec() instead like Oleg initially suggested.

>From 04d7f15afe14bbd0ede066b413214ceb2bcf7113 Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Sat, 4 Feb 2012 10:47:10 +0100
Subject: [PATCH] exec: fix use-after-free bug in setup_new_exec()

Setting the task name is done within setup_new_exec() by accessing
bprm->filename. However this happens after flush_old_exec().
This may result in a use after free bug, flush_old_exec() may
"complete" vfork_done, which will wake up the parent which in turn
may free the passed in filename.
To fix this add a new tcomm field in struct linux_binprm which
contains the now early generated task name until it is used.

Fixes this bug on s390:

[    9.642907] Unable to handle kernel pointer dereference at virtual kernel address 0000000039768000
[    9.642918] Oops: 0011 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[    9.642934] Modules linked in: qeth_l3 lcs ctcm fsm vmur qeth ccwgroup autofs4 [last unloaded: scsi_wait_scan]
[    9.642965] CPU: 0 Not tainted 3.3.0-rc2-00037-gbd3ce7d-dirty #48
[    9.643011] Process kworker/u:3 (pid: 245, task: 000000003a3dc840, ksp: 0000000039453818)
[    9.643015] Krnl PSW : 0704000180000000 0000000000282e94 (setup_new_exec+0xa0/0x374)
[    9.643026]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[    9.643032] Krnl GPRS: 0000000030844dcb fffffffffffffffd 0000000039768000 0000000000000000
[    9.643039]            00000000000000cd 0000000000243c18 00000000000001f8 0000000039453dd0
[    9.643045]            000000003dc55220 0000000000000000 00000000006f4800 000000003dc55220
[    9.643051]            0000000000000000 00000000005dd958 00000000002830f0 0000000039453a18
[    9.643067] Krnl Code: 0000000000282e84: c0e5ffffff52        brasl   %r14,282d28
[    9.643079]            0000000000282e8a: e320b0c80004        lg      %r2,200(%r11)
[    9.643092]           #0000000000282e90: a7380000            lhi     %r3,0
[    9.643108]           >0000000000282e94: e31020000090        llgc    %r1,0(%r2)
[    9.643123]            0000000000282e9a: 41202001            la      %r2,1(%r2)
[    9.643162]            0000000000282e9e: 1241                ltr     %r4,%r1
[    9.643168]            0000000000282ea0: a7840018            brc     8,282ed0
[    9.643175]            0000000000282ea4: a74e002f            chi     %r4,47
[    9.643183] Call Trace:
[    9.643186] ([<0000000000282e2c>] setup_new_exec+0x38/0x374)
[    9.643192]  [<00000000002dd12e>] load_elf_binary+0x402/0x1bf4
[    9.643201]  [<0000000000280a42>] search_binary_handler+0x38e/0x5bc
[    9.643210]  [<0000000000282b6c>] do_execve_common+0x410/0x514
[    9.643218]  [<0000000000282cb6>] do_execve+0x46/0x58
[    9.643225]  [<00000000005bce58>] kernel_execve+0x28/0x70
[    9.643236]  [<000000000014ba2e>] ____call_usermodehelper+0x102/0x140
[    9.643245]  [<00000000005bc8da>] kernel_thread_starter+0x6/0xc
[    9.643254]  [<00000000005bc8d4>] kernel_thread_starter+0x0/0xc
[    9.643264] INFO: lockdep is turned off.
[    9.643269] Last Breaking-Event-Address:
[    9.643275]  [<00000000002830f0>] setup_new_exec+0x2fc/0x374
[    9.643311]
[    9.643315] Kernel panic - not syncing: Fatal exception: panic_on_oops

Reported-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 fs/exec.c               |   33 +++++++++++++++++----------------
 include/linux/binfmts.h |    3 ++-
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index aeb135c..92ce83a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1071,6 +1071,21 @@ void set_task_comm(struct task_struct *tsk, char *buf)
 	perf_event_comm(tsk);
 }
 
+static void filename_to_taskname(char *tcomm, const char *fn, unsigned int len)
+{
+	int i, ch;
+
+	/* Copies the binary name from after last slash */
+	for (i = 0; (ch = *(fn++)) != '\0';) {
+		if (ch == '/')
+			i = 0; /* overwrite what we wrote */
+		else
+			if (i < len - 1)
+				tcomm[i++] = ch;
+	}
+	tcomm[i] = '\0';
+}
+
 int flush_old_exec(struct linux_binprm * bprm)
 {
 	int retval;
@@ -1085,6 +1100,7 @@ int flush_old_exec(struct linux_binprm * bprm)
 
 	set_mm_exe_file(bprm->mm, bprm->file);
 
+	filename_to_taskname(bprm->tcomm, bprm->filename, sizeof(bprm->tcomm));
 	/*
 	 * Release all of the old mmap stuff
 	 */
@@ -1116,10 +1132,6 @@ EXPORT_SYMBOL(would_dump);
 
 void setup_new_exec(struct linux_binprm * bprm)
 {
-	int i, ch;
-	const char *name;
-	char tcomm[sizeof(current->comm)];
-
 	arch_pick_mmap_layout(current->mm);
 
 	/* This is the point of no return */
@@ -1130,18 +1142,7 @@ void setup_new_exec(struct linux_binprm * bprm)
 	else
 		set_dumpable(current->mm, suid_dumpable);
 
-	name = bprm->filename;
-
-	/* Copies the binary name from after last slash */
-	for (i=0; (ch = *(name++)) != '\0';) {
-		if (ch == '/')
-			i = 0; /* overwrite what we wrote */
-		else
-			if (i < (sizeof(tcomm) - 1))
-				tcomm[i++] = ch;
-	}
-	tcomm[i] = '\0';
-	set_task_comm(current, tcomm);
+	set_task_comm(current, bprm->tcomm);
 
 	/* Set the new mm task size. We have to do that late because it may
 	 * depend on TIF_32BIT which is only updated in flush_thread() on
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index fd88a39..0092102 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -18,7 +18,7 @@ struct pt_regs;
 #define BINPRM_BUF_SIZE 128
 
 #ifdef __KERNEL__
-#include <linux/list.h>
+#include <linux/sched.h>
 
 #define CORENAME_MAX_SIZE 128
 
@@ -58,6 +58,7 @@ struct linux_binprm {
 	unsigned interp_flags;
 	unsigned interp_data;
 	unsigned long loader, exec;
+	char tcomm[TASK_COMM_LEN];
 };
 
 #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0
-- 
1.7.9


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-02-05  4:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-03 15:44 cgroup_release_agent() with call_usermodehelper() with UMH_WAIT_EXEC may crash Heiko Carstens
2012-02-03 16:04 ` Oleg Nesterov
2012-02-03 16:48   ` Linus Torvalds
2012-02-04 10:03     ` Heiko Carstens
2012-02-04 15:16       ` Linus Torvalds
2012-02-05  4:12         ` Heiko Carstens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox