Re: [PATCH 1/3] exec: Make do_coredump more resilient to recursivecrashes (v8)

[Neil Horman]

Reposting with Oleg CC'd for him to ACK

On Fri, Jul 03, 2009 at 06:50:08AM -0400, Neil Horman wrote:
> core_pattern: Change how we detect recursive dumps with core_pattern pipes
> 
> Change how we detect recursive dumps.  Currently we have a mechanism by which
> we try to compare pathnames of the crashing process to the core_pattern path.
> This is broken for a dozen reasons, and just doesn't work in any sort of robust
> way.  I'm replacing it with the use of a 0 RLIMIT_CORE value.  Since helper
> apps set RLIMIT_CORE to zero, we don't write out core files for any process with
> that particular limit set.  It the core_pattern is a pipe, any non-zero limit is
> translated to RLIM_INFINITY.  This allows complete dumps to be captured, but
> prevents infinite recursion in the event that the core_pattern process itself
> crashes.
> 
> Signed-off-by: Neil Horman <nhorman@xxxxxxxxxxxxx>
> Reported-by: Earl Chew <earl_chew@xxxxxxxxxxx>
> 
> 
>  exec.c |   43 +++++++++++++++++++++----------------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 9e05bd8..9defd20 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1776,35 +1776,34 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
>   	lock_kernel();
>  	ispipe = format_corename(corename, signr);
>  	unlock_kernel();
> -	/*
> -	 * Don't bother to check the RLIMIT_CORE value if core_pattern points
> -	 * to a pipe.  Since we're not writing directly to the filesystem
> -	 * RLIMIT_CORE doesn't really apply, as no actual core file will be
> -	 * created unless the pipe reader choses to write out the core file
> -	 * at which point file size limits and permissions will be imposed
> -	 * as it does with any other process
> -	 */
> +
>   	if (ispipe) {
> +		if (core_limit == 0) {
> +			 /*
> +			  * Normally core limits are irrelevant to pipes, since
> +			  * we're not writing to the file system, but we use
> +			  * core_limit of 0 here as a speacial value. Any
> +			  * non-zero limit gets set to RLIM_INFINITY below, but
> +			  * a limit of 0 skips the dump.  This is a consistent
> +			  * way to catch recursive crashes.  We can still crash
> +			  * if the core_pattern binary sets RLIM_CORE =  !0
> +			  * but it runs as root, and can do lots of stupid things
> +			  * Note that we use task_tgid_vnr here to grab the pid of the
> +			  * process group leader.  That way we get the right pid if a thread
> +			  * in a multi-threaded core_pattern process dies.
> +			  */
> +			printk(KERN_WARNING "Process %d(%s) has RLIMIT_CORE set to 0\n",
> +			       task_tgid_vnr(current), current->comm);
> +			printk(KERN_WARNING "Aborting core\n");
> +			goto fail_unlock;
> +		}
> +
>  		helper_argv = argv_split(GFP_KERNEL, corename+1, &helper_argc);
>  		if (!helper_argv) {
>  			printk(KERN_WARNING "%s failed to allocate memory\n",
>  			       __func__);
>  			goto fail_unlock;
>  		}
> -		/* Terminate the string before the first option */
> -		delimit = strchr(corename, ' ');
> -		if (delimit)
> -			*delimit = '\0';
> -		delimit = strrchr(helper_argv[0], '/');
> -		if (delimit)
> -			delimit++;
> -		else
> -			delimit = helper_argv[0];
> -		if (!strcmp(delimit, current->comm)) {
> -			printk(KERN_NOTICE "Recursive core dump detected, "
> -					"aborting\n");
> -			goto fail_unlock;
> -		}
>  
>  		core_limit = RLIM_INFINITY;
>  
> --
> 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/
> 
--
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/