[Feature Request?] Inline compression of process core dumps

[Feature Request?] Inline compression of process core dumps

[Christopher S. Aker]

I've been trying to find a method for compressing process core dumps 
before they hit disk.

I ask because we've got some fairly large UML processes (1GB for some), 
and we're trying to capture dumps to help Jeff debug an evasive bug. 
Our systems use a small root partition and most of the other disk 
resources on the host are allocated towards the UMLs.

There are userspace solutions to this problem:  allowing the 
uncompressed core dump to spin out to disk and then coming in afterwards 
and doing the compression, or maybe even a compressed filesystem where 
the core dumps land, but I just thought I'd throw this out there since 
it seems it would be a useful feature :)

Thanks,
-Chris

Re: [Feature Request?] Inline compression of process core dumps

[Bill Rugolsky Jr.]

On Thu, Apr 12, 2007 at 11:52:38AM -0400, Christopher S. Aker wrote:
> I've been trying to find a method for compressing process core dumps 
> before they hit disk.
> 
> I ask because we've got some fairly large UML processes (1GB for some), 
> and we're trying to capture dumps to help Jeff debug an evasive bug. 
> Our systems use a small root partition and most of the other disk 
> resources on the host are allocated towards the UMLs.
> 
> There are userspace solutions to this problem:  allowing the 
> uncompressed core dump to spin out to disk and then coming in afterwards 
> and doing the compression, or maybe even a compressed filesystem where 
> the core dumps land, but I just thought I'd throw this out there since 
> it seems it would be a useful feature :)

See Documentation/kernel.txt for kernels >= 2.6.19:

core_pattern:

core_pattern is used to specify a core dumpfile pattern name.
. max length 128 characters; default value is "core"
. core_pattern is used as a pattern template for the output filename;
  certain string patterns (beginning with '%') are substituted with
  their actual values.
. backward compatibility with core_uses_pid:
        If core_pattern does not include "%p" (default does not)
        and core_uses_pid is set, then .PID will be appended to
        the filename.
. corename format specifiers:
        %<NUL>  '%' is dropped
        %%      output one '%'
        %p      pid
        %u      uid
        %g      gid
        %s      signal number
        %t      UNIX time of dump
        %h      hostname
        %e      executable filename
        %<OTHER> both are dropped
. If the first character of the pattern is a '|', the kernel will treat
  the rest of the pattern as a command to run.  The core dump will be
  written to the standard input of that program instead of to a file.


Regards,

	Bill Rugolsky

Re: [Feature Request?] Inline compression of process core dumps

[Guillaume Chazarain]

2007/4/12, Christopher S. Aker <caker@theshore.net>:
> I've been trying to find a method for compressing process core dumps
> before they hit disk.

This seems to be a good use of:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d025c9db7f31fc0554ce7fb2dfc78d35a77f3487

Regards.

-- 
Guillaume

Re: [Feature Request?] Inline compression of process core dumps

[Alan Cox]

> There are userspace solutions to this problem:  allowing the 
> uncompressed core dump to spin out to disk and then coming in afterwards 
> and doing the compression, or maybe even a compressed filesystem where 
> the core dumps land, but I just thought I'd throw this out there since 
> it seems it would be a useful feature :)

Indeed. So useful that in current kernels you can set the core dump path
to be

	"|application"

and it will call out to the helper. Take care with the helper as it will
get run for setuid apps, roots core dumps etc.

Alan

Re: [Feature Request?] Inline compression of process core dumps

[Bill Rugolsky Jr.]

On Thu, Apr 12, 2007 at 05:28:45PM +0100, Alan Cox wrote:
> > There are userspace solutions to this problem:  allowing the 
> > uncompressed core dump to spin out to disk and then coming in afterwards 
> > and doing the compression, or maybe even a compressed filesystem where 
> > the core dumps land, but I just thought I'd throw this out there since 
> > it seems it would be a useful feature :)
> 
> Indeed. So useful that in current kernels you can set the core dump path
> to be
> 
> 	"|application"
> 
> and it will call out to the helper. Take care with the helper as it will
> get run for setuid apps, roots core dumps etc.

The current functionality doesn't parse command line arguments into argv,
nor provide the % variable replacements in the environment, so it is
somewhat less useful than it could be.  I supposed that parsing the command
line introduces potential problems with file names that include whitespace.
It would probably be better to split the command-line on whitespace, then
replace variables in the argv[]?

fs/exec.c:
1507         if (corename[0] == '|') {
1508                 /* SIGPIPE can happen, but it's just never processed */
1509                 if(call_usermodehelper_pipe(corename+1, NULL, NULL, &file)) {
1510                         printk(KERN_INFO "Core dump to %s pipe failed\n",
1511                                corename);
1512                         goto fail_unlock;
1513                 }
1514                 ispipe = 1;
1515         } else
1516                 file = filp_open(corename,
1517                                  O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag,
1518                                  0600);


Regards,

	Bill Rugolsky

Re: [Feature Request?] Inline compression of process core dumps

[Andi Kleen]

"Bill Rugolsky Jr." <brugolsky@telemetry-investments.com> writes:
> 
> The current functionality doesn't parse command line arguments into argv,
> nor provide the % variable replacements in the environment, so it is
> somewhat less useful than it could be. 

That was intentional because all this information is in the core dump itself
or in the environment.

-Andi

Re: [Feature Request?] Inline compression of process core dumps

[Christopher S. Aker]

Alan Cox wrote:
 > Indeed. So useful that in current kernels you can set the core dump
 > path to be
 >
 > 	"|application"

Cool stuff!  However, it's not working (2.6.20.6):

	Core dump to |/home/caker/bin/dumper.pl.4442 pipe failed

even though...

	# cat /proc/sys/kernel/core_uses_pid
	0
	# cat /proc/sys/kernel/core_pattern
	|/home/caker/bin/dumper.pl

Looking at the code, it seems to me that format_corename() is appending 
.pid, regardless if !core_uses_pid and corename[0]=='|', in which case 
it creates an invalid path for call_usermodehelper_pipe().

Bug in the code, or bug in my methods?

-Chris

Re: [Feature Request?] Inline compression of process core dumps

[Randy Dunlap]

On Thu, 12 Apr 2007 22:22:18 -0400 Christopher S. Aker wrote:

> Alan Cox wrote:
>  > Indeed. So useful that in current kernels you can set the core dump
>  > path to be
>  >
>  > 	"|application"
> 
> Cool stuff!  However, it's not working (2.6.20.6):
> 
> 	Core dump to |/home/caker/bin/dumper.pl.4442 pipe failed
> 
> even though...
> 
> 	# cat /proc/sys/kernel/core_uses_pid
> 	0
> 	# cat /proc/sys/kernel/core_pattern
> 	|/home/caker/bin/dumper.pl
> 
> Looking at the code, it seems to me that format_corename() is appending 
> .pid, regardless if !core_uses_pid and corename[0]=='|', in which case 
> it creates an invalid path for call_usermodehelper_pipe().
> 
> Bug in the code, or bug in my methods?

What are you trying to dump?  is it a multi-thread group app,
not a "simple" app?  I ask because of this (I'm looking at 2.6.21-rc6)
<mm_users> reference (not that I know what that is):

	if (!pid_in_pattern
            && (core_uses_pid || atomic_read(&current->mm->mm_users) != 1)) {
		rc = snprintf(out_ptr, out_end - out_ptr,
			      ".%d", current->tgid);
		if (rc > out_end - out_ptr)
			goto out;
		out_ptr += rc;
	}

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [Feature Request?] Inline compression of process core dumps

[Christopher S. Aker]

Randy Dunlap wrote:
> On Thu, 12 Apr 2007 22:22:18 -0400 Christopher S. Aker wrote:
> 
>> Alan Cox wrote:
>>  > Indeed. So useful that in current kernels you can set the core dump
>>  > path to be
>>  >
>>  > 	"|application"
>>
>> Cool stuff!  However, it's not working (2.6.20.6):
>>
>> 	Core dump to |/home/caker/bin/dumper.pl.4442 pipe failed
>>
>> even though...
>>
>> 	# cat /proc/sys/kernel/core_uses_pid
>> 	0
>> 	# cat /proc/sys/kernel/core_pattern
>> 	|/home/caker/bin/dumper.pl
>>
>> Looking at the code, it seems to me that format_corename() is appending 
>> .pid, regardless if !core_uses_pid and corename[0]=='|', in which case 
>> it creates an invalid path for call_usermodehelper_pipe().
>>
>> Bug in the code, or bug in my methods?
> 
> What are you trying to dump?  is it a multi-thread group app,
> not a "simple" app?  I ask because of this (I'm looking at 2.6.21-rc6)
> <mm_users> reference (not that I know what that is):
> 
> 	if (!pid_in_pattern
>             && (core_uses_pid || atomic_read(&current->mm->mm_users) != 1)) {
> 		rc = snprintf(out_ptr, out_end - out_ptr,
> 			      ".%d", current->tgid);
> 		if (rc > out_end - out_ptr)
> 			goto out;
> 		out_ptr += rc;
> 	}

I saw that too, and unfortunately I don't know what what that condition 
represents, either.  It's the only other element in that if statement 
that could make it take that path, so I'm assuming that's part of the 
problem.

The process is a UML instance (skas mode, so at least a kernel, 
userspace, and io thread), which will generate a single, usable, core 
file just fine with a non-pipe core_pattern...

-Chris

Re: [Feature Request?] Inline compression of process core dumps

[Jeff Dike]

On Thu, Apr 12, 2007 at 10:57:37PM -0400, Christopher S. Aker wrote:
> The process is a UML instance (skas mode, so at least a kernel, 
> userspace, and io thread), which will generate a single, usable, core 
> file just fine with a non-pipe core_pattern...

Yeah, but can you get a core file without the .pid on the end?  I just
tried, with core_pattern == core and core_uses_pid == 0, and I still
got core.pid.

I can fix this on my end - just have to kill off a bunch of things
before aborting.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: [Feature Request?] Inline compression of process core dumps

[Alan Cox]

> I saw that too, and unfortunately I don't know what what that condition 
> represents, either.  It's the only other element in that if statement 
> that could make it take that path, so I'm assuming that's part of the 
> problem.

Multiple mm's mean multiple threads with a different set of mappings,
which would fit for UML. Either way there should be a check for !pipe
before appending the pid

Re: [Feature Request?] Inline compression of process core dumps

[Andi Kleen]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> > I saw that too, and unfortunately I don't know what what that condition 
> > represents, either.  It's the only other element in that if statement 
> > that could make it take that path, so I'm assuming that's part of the 
> > problem.
> 
> Multiple mm's mean multiple threads with a different set of mappings,
> which would fit for UML. Either way there should be a check for !pipe
> before appending the pid

Here's a patch. It just doesn't do any formatting for the pipe case.

-Andi

Fix core to pipe for multithreaded processes

I also removed the BKL around format_corename because it seems unneeded.

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux-2.6.21-rc3-test/fs/exec.c
===================================================================
--- linux-2.6.21-rc3-test.orig/fs/exec.c
+++ linux-2.6.21-rc3-test/fs/exec.c
@@ -1501,9 +1501,6 @@ int do_coredump(long signr, int exit_cod
 	 * lock_kernel() because format_corename() is controlled by sysctl, which
 	 * uses lock_kernel()
 	 */
- 	lock_kernel();
-	format_corename(corename, core_pattern, signr);
-	unlock_kernel();
  	if (corename[0] == '|') {
 		/* SIGPIPE can happen, but it's just never processed */
  		if(call_usermodehelper_pipe(corename+1, NULL, NULL, &file)) {
@@ -1512,10 +1509,12 @@ int do_coredump(long signr, int exit_cod
  			goto fail_unlock;
  		}
 		ispipe = 1;
- 	} else
+ 	} else {
+		format_corename(corename, core_pattern, signr);
  		file = filp_open(corename,
 				 O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag,
 				 0600);
+	}
 	if (IS_ERR(file))
 		goto fail_unlock;
 	inode = file->f_path.dentry->d_inode;

Re: [Feature Request?] Inline compression of process core dumps

[Randy Dunlap]

Andi Kleen wrote:
> Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> 
>>> I saw that too, and unfortunately I don't know what what that condition 
>>> represents, either.  It's the only other element in that if statement 
>>> that could make it take that path, so I'm assuming that's part of the 
>>> problem.
>> Multiple mm's mean multiple threads with a different set of mappings,
>> which would fit for UML. Either way there should be a check for !pipe
>> before appending the pid
> 
> Here's a patch. It just doesn't do any formatting for the pipe case.
> 
> -Andi
> 
> Fix core to pipe for multithreaded processes
> 
> I also removed the BKL around format_corename because it seems unneeded.

so either the comment just above lock_kernel() should be removed, or
it's correct and lock_kernel()/unlock_kernel() should stay.

	/*
	 * lock_kernel() because format_corename() is controlled by sysctl, which
	 * uses lock_kernel()
	 */
 	lock_kernel();
	format_corename(corename, core_pattern, signr);
	unlock_kernel();


> Signed-off-by: Andi Kleen <ak@suse.de>
> 
> Index: linux-2.6.21-rc3-test/fs/exec.c
> ===================================================================
> --- linux-2.6.21-rc3-test.orig/fs/exec.c
> +++ linux-2.6.21-rc3-test/fs/exec.c
> @@ -1501,9 +1501,6 @@ int do_coredump(long signr, int exit_cod
>  	 * lock_kernel() because format_corename() is controlled by sysctl, which
>  	 * uses lock_kernel()
>  	 */
> - 	lock_kernel();
> -	format_corename(corename, core_pattern, signr);
> -	unlock_kernel();
>   	if (corename[0] == '|') {
>  		/* SIGPIPE can happen, but it's just never processed */
>   		if(call_usermodehelper_pipe(corename+1, NULL, NULL, &file)) {
> @@ -1512,10 +1509,12 @@ int do_coredump(long signr, int exit_cod
>   			goto fail_unlock;
>   		}
>  		ispipe = 1;
> - 	} else
> + 	} else {
> +		format_corename(corename, core_pattern, signr);
>   		file = filp_open(corename,
>  				 O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag,
>  				 0600);
> +	}
>  	if (IS_ERR(file))
>  		goto fail_unlock;
>  	inode = file->f_path.dentry->d_inode;


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [Feature Request?] Inline compression of process core dumps

[Andi Kleen]

> so either the comment just above lock_kernel() should be removed, or
> it's correct and lock_kernel()/unlock_kernel() should stay.

Ah somehow missed the comment. Ok so it's better to keep it, although
there would be probably better ways to protect the pattern.

-Andi

Re: [Feature Request?] Inline compression of process core dumps

[Alan Cox]

> > Here's a patch. It just doesn't do any formatting for the pipe case.

I don't see a reaosn for removing the formatting features. There are ways
that can usefully be used, although not many. It's also messier than the
patch I posted while less functional IMHO.

Alan

Re: [Feature Request?] Inline compression of process core dumps

[Andi Kleen]

On Fri, Apr 13, 2007 at 05:17:27PM +0100, Alan Cox wrote:
> > > Here's a patch. It just doesn't do any formatting for the pipe case.
> 
> I don't see a reaosn for removing the formatting features. There are ways
> that can usefully be used, although not many. It's also messier than the
> patch I posted while less functional IMHO.

To make formatting for pipes useful you would need to split arguments.
I chose not to do it because all the information in the format can be gotten
in other ways already -- either from the coredump or from the environment

-Andi

Re: [Feature Request?] Inline compression of process core dumps

[Alan Cox]

> Looking at the code, it seems to me that format_corename() is appending 
> .pid, regardless if !core_uses_pid and corename[0]=='|', in which case 
> it creates an invalid path for call_usermodehelper_pipe().
> 
> Bug in the code, or bug in my methods?

This looks somewhat better and might do the trick. Also fixes a very very
obscure security corner case. If you change core pattern to start with
the program name then the user can run a program called "|myevilhack" as
it stands. The patch checks for "|" in the pattern not the output and
doesn't nail a pid on to a piped name.

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --exclude-from /usr/src/exclude --new-file --recursive linux.vanilla-2.6.21-rc6-mm1/fs/exec.c linux-2.6.21-rc6-mm1/fs/exec.c
--- linux.vanilla-2.6.21-rc6-mm1/fs/exec.c	2007-04-12 14:15:05.000000000 +0100
+++ linux-2.6.21-rc6-mm1/fs/exec.c	2007-04-13 13:11:20.709835952 +0100
@@ -1265,13 +1265,17 @@
  * name into corename, which must have space for at least
  * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator.
  */
-static void format_corename(char *corename, const char *pattern, long signr)
+static int format_corename(char *corename, const char *pattern, long signr)
 {
 	const char *pat_ptr = pattern;
 	char *out_ptr = corename;
 	char *const out_end = corename + CORENAME_MAX_SIZE;
 	int rc;
 	int pid_in_pattern = 0;
+	int ispipe = 0;
+	
+	if (*pattern == '|')
+		ispipe = 1;
 
 	/* Repeat as long as we have more pattern to process and more output
 	   space */
@@ -1362,8 +1366,8 @@
 	 *
 	 * If core_pattern does not include a %p (as is the default)
 	 * and core_uses_pid is set, then .%pid will be appended to
-	 * the filename */
-	if (!pid_in_pattern
+	 * the filename. Do not do this for piped commands. */
+	if (!ispipe && !pid_in_pattern
             && (core_uses_pid || atomic_read(&current->mm->mm_users) != 1)) {
 		rc = snprintf(out_ptr, out_end - out_ptr,
 			      ".%d", current->tgid);
@@ -1371,8 +1375,9 @@
 			goto out;
 		out_ptr += rc;
 	}
-      out:
+out:
 	*out_ptr = 0;
+	return ispipe;
 }
 
 static void zap_process(struct task_struct *start)
@@ -1523,16 +1528,15 @@
 	 * uses lock_kernel()
 	 */
  	lock_kernel();
-	format_corename(corename, core_pattern, signr);
+	ispipe = format_corename(corename, core_pattern, signr);
 	unlock_kernel();
- 	if (corename[0] == '|') {
+ 	if (ispipe) {
 		/* SIGPIPE can happen, but it's just never processed */
  		if(call_usermodehelper_pipe(corename+1, NULL, NULL, &file)) {
  			printk(KERN_INFO "Core dump to %s pipe failed\n",
 			       corename);
  			goto fail_unlock;
  		}
-		ispipe = 1;
  	} else
  		file = filp_open(corename,
 				 O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag,

Re: [Feature Request?] Inline compression of process core dumps

[Christopher S. Aker]

Alan Cox wrote:
>> Looking at the code, it seems to me that format_corename() is appending 
>> .pid, regardless if !core_uses_pid and corename[0]=='|', in which case 
>> it creates an invalid path for call_usermodehelper_pipe().
>>
>> Bug in the code, or bug in my methods?
> 
> This looks somewhat better and might do the trick. Also fixes a very very
> obscure security corner case. If you change core pattern to start with
> the program name then the user can run a program called "|myevilhack" as
> it stands. The patch checks for "|" in the pattern not the output and
> doesn't nail a pid on to a piped name.

<snip>

Works great now.  Queue this sucker up!

	# cat /proc/sys/kernel/core_pattern
	|/home/caker/bin/dumper.pl
	# ./linux
	<blah blah>
	Segmentation fault (core dumped)
	# file /tmp/dumper.out
	/tmp/dumper.out: ELF 32-bit LSB core file Intel 80386, version 1 
(SYSV), SVR4-style

Thanks for everyone's help.

-Chris