2006-08-14 11:27:55

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern


Using the infrastructure created in previous patches implement support
to pipe core dumps into programs.

This is done by overloading the existing core_pattern sysctl
with a new syntax:

|program

When the first character of the pattern is a '|' the kernel will
instead threat 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.

This is useful for having automatic core dump analysis without
filling up disks. The program can do some simple analysis and save only
a summary of the core dump.

The core dump proces will run with the privileges and in the name space
of the process that caused the core dump.

I also increased the core pattern size to 128 bytes so that
longer command lines fit.

Most of the changes comes from allowing core dumps without seeks.
They are fairly straight forward though.

One small incompatibility is that if someone had a core pattern previously that
started with '|' they will get suddenly new behaviour. I think that's
unlikely to be a real problem though.

Signed-off-by: Andi Kleen <[email protected]>

Index: linux/fs/exec.c
===================================================================
--- linux.orig/fs/exec.c
+++ linux/fs/exec.c
@@ -58,7 +58,7 @@
#endif

int core_uses_pid;
-char core_pattern[65] = "core";
+char core_pattern[128] = "core";
int suid_dumpable = 0;

EXPORT_SYMBOL(suid_dumpable);
@@ -1475,6 +1475,7 @@ int do_coredump(long signr, int exit_cod
int retval = 0;
int fsuid = current->fsuid;
int flag = 0;
+ int ispipe = 0;

binfmt = current->binfmt;
if (!binfmt || !binfmt->core_dump)
@@ -1516,22 +1517,34 @@ int do_coredump(long signr, int exit_cod
lock_kernel();
format_corename(corename, core_pattern, signr);
unlock_kernel();
- file = filp_open(corename, O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag, 0600);
+ if (corename[0] == '|') {
+ /* 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, 0600);
if (IS_ERR(file))
goto fail_unlock;
inode = file->f_dentry->d_inode;
if (inode->i_nlink > 1)
goto close_fail; /* multiple links - don't dump */
- if (d_unhashed(file->f_dentry))
+ if (!ispipe && d_unhashed(file->f_dentry))
goto close_fail;

- if (!S_ISREG(inode->i_mode))
+ /* AK: actually i see no reason to not allow this for named pipes etc.,
+ but keep the previous behaviour for now. */
+ if (!ispipe && !S_ISREG(inode->i_mode))
goto close_fail;
if (!file->f_op)
goto close_fail;
if (!file->f_op->write)
goto close_fail;
- if (do_truncate(file->f_dentry, 0, 0, file) != 0)
+ if (!ispipe && do_truncate(file->f_dentry, 0, 0, file) != 0)
goto close_fail;

retval = binfmt->core_dump(signr, regs, file);
Index: linux/fs/binfmt_elf.c
===================================================================
--- linux.orig/fs/binfmt_elf.c
+++ linux/fs/binfmt_elf.c
@@ -1153,11 +1153,23 @@ static int dump_write(struct file *file,

static int dump_seek(struct file *file, loff_t off)
{
- if (file->f_op->llseek) {
- if (file->f_op->llseek(file, off, 0) != off)
+ if (file->f_op->llseek && file->f_op->llseek != no_llseek) {
+ if (file->f_op->llseek(file, off, 1) != off)
return 0;
- } else
- file->f_pos = off;
+ } else {
+ char *buf = (char *)get_zeroed_page(GFP_KERNEL);
+ if (!buf)
+ return 0;
+ while (off > 0) {
+ unsigned long n = off;
+ if (n > PAGE_SIZE)
+ n = PAGE_SIZE;
+ if (!dump_write(file, buf, n))
+ return 0;
+ off -= n;
+ }
+ free_page((unsigned long)buf);
+ }
return 1;
}

@@ -1205,30 +1217,32 @@ static int notesize(struct memelfnote *e
return sz;
}

-#define DUMP_WRITE(addr, nr) \
- do { if (!dump_write(file, (addr), (nr))) return 0; } while(0)
-#define DUMP_SEEK(off) \
- do { if (!dump_seek(file, (off))) return 0; } while(0)
+#define DUMP_WRITE(addr, nr, foffset) \
+ do { if (!dump_write(file, (addr), (nr))) return 0; *foffset += (nr); } while(0)

-static int writenote(struct memelfnote *men, struct file *file)
+static int alignfile(struct file *file, unsigned long *foffset)
{
- struct elf_note en;
+ char buf[4] = { 0, };
+ DUMP_WRITE(buf, roundup(*foffset, 4) - *foffset, foffset);
+ return 1;
+}

+static int writenote(struct memelfnote *men, struct file *file, unsigned long *foffset)
+{
+ struct elf_note en;
en.n_namesz = strlen(men->name) + 1;
en.n_descsz = men->datasz;
en.n_type = men->type;

- DUMP_WRITE(&en, sizeof(en));
- DUMP_WRITE(men->name, en.n_namesz);
- /* XXX - cast from long long to long to avoid need for libgcc.a */
- DUMP_SEEK(roundup((unsigned long)file->f_pos, 4)); /* XXX */
- DUMP_WRITE(men->data, men->datasz);
- DUMP_SEEK(roundup((unsigned long)file->f_pos, 4)); /* XXX */
+ DUMP_WRITE(&en, sizeof(en), foffset);
+ DUMP_WRITE(men->name, en.n_namesz, foffset);
+ if (!alignfile(file, foffset)) return 0;
+ DUMP_WRITE(men->data, men->datasz, foffset);
+ if (!alignfile(file, foffset)) return 0;

return 1;
}
#undef DUMP_WRITE
-#undef DUMP_SEEK

#define DUMP_WRITE(addr, nr) \
if ((size += (nr)) > limit || !dump_write(file, (addr), (nr))) \
@@ -1428,7 +1442,7 @@ static int elf_core_dump(long signr, str
int i;
struct vm_area_struct *vma;
struct elfhdr *elf = NULL;
- off_t offset = 0, dataoff;
+ off_t offset = 0, dataoff, foffset;
unsigned long limit = current->signal->rlim[RLIMIT_CORE].rlim_cur;
int numnote;
struct memelfnote *notes = NULL;
@@ -1572,7 +1586,8 @@ static int elf_core_dump(long signr, str
DUMP_WRITE(&phdr, sizeof(phdr));
}

- /* Page-align dumped data */
+ foffset = offset;
+
dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE);

/* Write program headers for segments dump */
@@ -1597,6 +1612,7 @@ static int elf_core_dump(long signr, str
phdr.p_align = ELF_EXEC_PAGESIZE;

DUMP_WRITE(&phdr, sizeof(phdr));
+ foffset += sizeof(phdr);
}

#ifdef ELF_CORE_WRITE_EXTRA_PHDRS
@@ -1605,7 +1621,7 @@ static int elf_core_dump(long signr, str

/* write out the notes section */
for (i = 0; i < numnote; i++)
- if (!writenote(notes + i, file))
+ if (!writenote(notes + i, file, &foffset))
goto end_coredump;

/* write out the thread status notes section */
@@ -1614,11 +1630,12 @@ static int elf_core_dump(long signr, str
list_entry(t, struct elf_thread_status, list);

for (i = 0; i < tmp->num_notes; i++)
- if (!writenote(&tmp->notes[i], file))
+ if (!writenote(&tmp->notes[i], file, &foffset))
goto end_coredump;
}
-
- DUMP_SEEK(dataoff);
+
+ /* Align to page */
+ DUMP_SEEK(dataoff - foffset);

for (vma = current->mm->mmap; vma != NULL; vma = vma->vm_next) {
unsigned long addr;
@@ -1634,10 +1651,10 @@ static int elf_core_dump(long signr, str

if (get_user_pages(current, current->mm, addr, 1, 0, 1,
&page, &vma) <= 0) {
- DUMP_SEEK(file->f_pos + PAGE_SIZE);
+ DUMP_SEEK(PAGE_SIZE);
} else {
if (page == ZERO_PAGE(addr)) {
- DUMP_SEEK(file->f_pos + PAGE_SIZE);
+ DUMP_SEEK(PAGE_SIZE);
} else {
void *kaddr;
flush_cache_page(vma, addr,
@@ -1661,13 +1678,6 @@ static int elf_core_dump(long signr, str
ELF_CORE_WRITE_EXTRA_DATA;
#endif

- if ((off_t)file->f_pos != offset) {
- /* Sanity check */
- printk(KERN_WARNING
- "elf_core_dump: file->f_pos (%ld) != offset (%ld)\n",
- (off_t)file->f_pos, offset);
- }
-
end_coredump:
set_fs(fs);

Index: linux/kernel/sysctl.c
===================================================================
--- linux.orig/kernel/sysctl.c
+++ linux/kernel/sysctl.c
@@ -293,7 +293,7 @@ static ctl_table kern_table[] = {
.ctl_name = KERN_CORE_PATTERN,
.procname = "core_pattern",
.data = core_pattern,
- .maxlen = 64,
+ .maxlen = 128,
.mode = 0644,
.proc_handler = &proc_dostring,
.strategy = &sysctl_string,


2006-08-15 00:43:24

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Mon, 14 Aug 2006 13:27:32 +0200 (CEST)
Andi Kleen <[email protected]> wrote:

> The core dump proces will run with the privileges and in the name space
> of the process that caused the core dump.

Don't think so. __call_usermodehelper() is executed by the khelper kernel thread.

2006-08-15 06:04:22

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Mon, Aug 14, 2006 at 05:43:19PM -0700, Andrew Morton wrote:
> On Mon, 14 Aug 2006 13:27:32 +0200 (CEST)
> Andi Kleen <[email protected]> wrote:
>
> > The core dump proces will run with the privileges and in the name space
> > of the process that caused the core dump.
>
> Don't think so. __call_usermodehelper() is executed by the khelper kernel thread.

Yes, you're right the comment is wrong. It does run as root instead
with global name space. I needed that because otherwise
it needed a global writable directory for the core dumps.

Feel free to change it in your version.

That happens when you write the description months later than the code ...

-Andi

2006-08-16 08:44:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Mon, Aug 14, 2006 at 01:27:32PM +0200, Andi Kleen wrote:
>
> Using the infrastructure created in previous patches implement support
> to pipe core dumps into programs.
>
> This is done by overloading the existing core_pattern sysctl
> with a new syntax:
>
> |program

Very nice, do you happen to have a program that can accept this kind of
input for crash dumps? I'm guessing that the embedded people will
really want this functionality.

thanks,

greg k-h

2006-08-16 09:18:10

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Wed, 16 Aug 2006 01:43:54 -0700
Greg KH <[email protected]> wrote:

> On Mon, Aug 14, 2006 at 01:27:32PM +0200, Andi Kleen wrote:
> >
> > Using the infrastructure created in previous patches implement support
> > to pipe core dumps into programs.
> >
> > This is done by overloading the existing core_pattern sysctl
> > with a new syntax:
> >
> > |program
>
> Very nice, do you happen to have a program that can accept this kind of
> input for crash dumps? I'm guessing that the embedded people will
> really want this functionality.

I had a cheesy demo/prototype. Basically it wrote the dump to a file again,
ran gdb on it to get a backtrace and wrote the summary to a shared directory.
Then there was a simple CGI script to generate a "top 10" crashes
HTML listing.

Unfortunately this still had the disadvantage to needing full disk space
for a dump except for deleting it afterwards (in fact it was worse because
over the pipe holes didn't work so if you have a holey address map it would
require more space).

Fortunately gdb seems to be happy to handle /proc/pid/fd/xxx input pipes
as cores (at least it worked with zsh's =(cat core) syntax), so it would be
likely possible to do it without temporary space with a simple wrapper that
calls it in the right way. I ran out of time before doing that though.

The demo prototype scripts weren't very good. If there is really interest
I can dig them out (they are currently on a laptop disk on the desk
with the laptop itself being in service), but I would recommend to
rewrite them for any serious application of this and fix the disk space problem.

Also to be really useful it should probably find a way to automatically
fetch the debuginfos (I cheated and just installed them in advance)

If nobody else does it I can probably do the rewrite myself again at some point.

My hope at some point was that desktops would support it in their
builtin crash reporters, but at least the KDE people I talked
too seemed to be happy with their user space only solution.

-Andi

2006-08-16 18:10:35

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Wed, 16 Aug 2006 11:18:01 +0200
Andi Kleen <[email protected]> wrote:

> > Very nice, do you happen to have a program that can accept this kind of
> > input for crash dumps? I'm guessing that the embedded people will
> > really want this functionality.
>
> I had a cheesy demo/prototype. Basically it wrote the dump to a file again,
> ran gdb on it to get a backtrace and wrote the summary to a shared directory.
> Then there was a simple CGI script to generate a "top 10" crashes
> HTML listing.
>
> Unfortunately this still had the disadvantage to needing full disk space
> for a dump except for deleting it afterwards (in fact it was worse because
> over the pipe holes didn't work so if you have a holey address map it would
> require more space).
>
> Fortunately gdb seems to be happy to handle /proc/pid/fd/xxx input pipes
> as cores (at least it worked with zsh's =(cat core) syntax), so it would be
> likely possible to do it without temporary space with a simple wrapper that
> calls it in the right way. I ran out of time before doing that though.
>
> The demo prototype scripts weren't very good. If there is really interest
> I can dig them out (they are currently on a laptop disk on the desk
> with the laptop itself being in service), but I would recommend to
> rewrite them for any serious application of this and fix the disk space problem.
>
> Also to be really useful it should probably find a way to automatically
> fetch the debuginfos (I cheated and just installed them in advance)
>
> If nobody else does it I can probably do the rewrite myself again at some point.
>
> My hope at some point was that desktops would support it in their
> builtin crash reporters, but at least the KDE people I talked
> too seemed to be happy with their user space only solution.

It doens't sounds like there's particularly strong userspace "pull" for
this feature?

2006-08-17 09:46:43

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

> It doens't sounds like there's particularly strong userspace "pull" for
> this feature?

Several people from the embedded area wrote me privately
it would be useful for them. Also I think once it's in the main kernel
there will be more incentive for user space to use it and I'm optimistic
it will get some adoption (ok I guess I should write some better
documentation, but there was no obvious place for it)

-Andi

2006-08-17 11:09:14

by Alan

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

Ar Iau, 2006-08-17 am 11:46 +0200, ysgrifennodd Andi Kleen:
> Several people from the embedded area wrote me privately
> it would be useful for them. Also I think once it's in the main kernel
> there will be more incentive for user space to use it and I'm optimistic
> it will get some adoption (ok I guess I should write some better
> documentation, but there was no obvious place for it)

I don't believe that piping as such as neccessarily the right model, but
the ability to intercept and processes core dumps from user space is
asked for by many enterprise users as well. They want to know about,
capture, analyse and process core dumps, often centrally and in
automated form.

Alan

2006-08-18 05:30:23

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Thu, 17 Aug 2006 12:27:44 +0100
Alan Cox <[email protected]> wrote:

> Ar Iau, 2006-08-17 am 11:46 +0200, ysgrifennodd Andi Kleen:
> > Several people from the embedded area wrote me privately
> > it would be useful for them. Also I think once it's in the main kernel
> > there will be more incentive for user space to use it and I'm optimistic
> > it will get some adoption (ok I guess I should write some better
> > documentation, but there was no obvious place for it)
>
> I don't believe that piping as such as neccessarily the right model, but
> the ability to intercept and processes core dumps from user space is
> asked for by many enterprise users as well. They want to know about,
> capture, analyse and process core dumps, often centrally and in
> automated form.
>

OK, fair enough.

Now let's think about security. Patches against ptrace, coredump and
procfs give me the creeps because we've had (relatively) so many problems
in those areas in the past.

So I'd suggest that we should look at this code and think about it in a
really twisted fashion - does it open any exploits? I can't think of any,
which is worth practically zero, but I don't see how this differs from
/proc/sys/kernel/modprobe.

But still. Is this code secure?

2006-08-18 08:30:01

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

> But still. Is this code secure?

Any auditing from third parties appreciated.

I don't know of any obvious flaws (at least assuming the pipe handler
isn't insecure code), but then I'm biased.

-Andi

2006-08-18 12:02:09

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Fri, 2006-08-18 at 10:29 +0200, Andi Kleen wrote:
> > But still. Is this code secure?
>
> Any auditing from third parties appreciated.
>
> I don't know of any obvious flaws (at least assuming the pipe handler
> isn't insecure code), but then I'm biased.

one angle is... is the userspace handler trusted code by root? Or is it
allowed to be per user/user defined binary? In the later case there are
all kinds of DoS scenarios possible; in the former it's root doing that
to himself ;)

--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com

2006-08-18 12:10:29

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] [3/3] Support piping into commands in /proc/sys/kernel/core_pattern

On Fri, 2006-08-18 at 14:01 +0200, Arjan van de Ven wrote:
> On Fri, 2006-08-18 at 10:29 +0200, Andi Kleen wrote:
> > > But still. Is this code secure?
> >
> > Any auditing from third parties appreciated.
> >
> > I don't know of any obvious flaws (at least assuming the pipe handler
> > isn't insecure code), but then I'm biased.
>
> one angle is... is the userspace handler trusted code by root? Or is it
> allowed to be per user/user defined binary? In the later case there are
> all kinds of DoS scenarios possible; in the former it's root doing that
> to himself ;)

so one angle that needs to be resolved is the selinux interaction, where
root!=root to some degree. If "any root" can control the sysctl, then
this could lead to information leaks, or worse, to a "limited root" to
"full root" escalation...

>
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com