Re: [PATCH] allow core_patten to be a FIFO, kernel 2.6.14

[Andi Kleen]

Devin Bayer <devin@xxxxxxxxxxxxx> writes:
> 
> I'm looking for comments, testing and inclusion in the next release.
> I have tested it in UML and one i686 build. The coredump files
> produced were valid.

I did a similar patch some time ago, but it allowed to execute a program
instead of allowing fifos with the core on stdin. IMHO that's a better
usage model because it doesn't require a daemon (and if you want one
you can use a trivial forwarder) 

I didn't post it because it still needed some cleanup and
double checking of a few corner cases and ran out of time for that.

I agree it's very useful. In my case the idea was to do 
automatic crash reporting. I wrote some simpleminded backup
scripts for that.

If there is interest I can dig it out. I think it was already
in better shape than your patch ;-)

>  {
> -	if (file->f_op->llseek) {
> +	if (off == file->f_pos) +		return 1;
> +	if (file->f_op->llseek == no_llseek && off > file->f_pos) {
> +		int nr = off - file->f_pos;
> +		char zeros[nr];
> +
> +		memset(zeros,0,nr);
> +		return dump_write(file, zeros, nr);

That's a exploitable root hole and a likely crash I think.

> +	    if (do_truncate(file->f_dentry, 0) != 0)
> +		    goto close_fail;
> +	}
>  	retval = binfmt->core_dump(signr, regs, file);
>  +	if(S_ISFIFO(inode->i_mode))
> +	    spin_unlock(&fifo_core_lock);

You're holding a spinlock over operations that can sleep like
write or truncate? That's totally wrong.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/