Re: Kernel panic because of wrong contents in core_pattern

[Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>]

Am Freitag, 15. November 2019, 14:27:40 CET schrieb Al Viro:
> On Fri, Nov 15, 2019 at 02:01:55PM +0100, Dietmar Hahn wrote:
> 
> > Later a user tool dumped with SIGSEGV and the linux system crashed.
> > I investigated the crash dump and found the cause.
> > 
> > Via format_corename() in fs/coredump.c the helper_argv[] with 3 entries is
> > created and helper_argv[0] == "" (because of the ' ' after the '|')
> > ispipe is set to 1.
> > Later in call_usermodehelper_setup():
> >   sub_info->path = path;  == helper_argv[0] == ""
> > This leads in call_usermodehelper_exec() to:
> >   if (strlen(sub_info->path) == 0)
> >                 goto out;
> > with a return value of 0.
> > But no pipe is created and thus cprm.file == NULL.
> > This leads in file_start_write() to the panic because of dereferencing
> >  file_inode(file)->i_mode)
> > 
> > I'am not sure what's the best way to fix this so I've no patch.
> > Thanks.
> 
> Check in the caller of format_corename() for **argv being '\0' and fail
> if it is?  I mean, turn that
>                 if (ispipe < 0) {
>                         printk(KERN_WARNING "format_corename failed\n");
>                         printk(KERN_WARNING "Aborting core\n");
>                         goto fail_unlock;
>                 }   
> in there into
> 		if (ispipe < 0 || !**argv) {
>                         printk(KERN_WARNING "format_corename failed\n");
>                         printk(KERN_WARNING "Aborting core\n");
>                         goto fail_unlock;
>                 }

Unfortunately this doesn't work because argv[0] is always 0 in case of ispipe
in format_corename():
	if (ispipe) {
		int argvs = sizeof(core_pattern) / 2;
		(*argv) = kmalloc_array(argvs, sizeof(**argv), GFP_KERNEL);
		if (!(*argv))
			return -ENOMEM;
		(*argv)[(*argc)++] = 0;
		++pat_ptr;
	}

The manpage says: The program must be ..., and must immediately
follow the '|' character.
Why not check this in format_corename(), maybe:

@@ -211,6 +211,8 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
                        return -ENOMEM;
                (*argv)[(*argc)++] = 0;
                ++pat_ptr;
+               if (isspace(*pat_ptr))
+                       return -EINVAL;
        }

Dietmar.