linux-kernel - Re: [PATCH] argv_split: Return NULL if argument contains nonon-whitespace.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 15 Sep 2013 09:10:53 +0900
From:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:	oleg@...hat.com
Cc:	viro@...iv.linux.org.uk, rostedt@...dmis.org, fweisbec@...il.com,
	mingo@...hat.com, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] argv_split: Return NULL if argument contains nonon-whitespace.

Oleg Nesterov wrote:
> > upon core dump because helper_argv[0] == NULL at
> >
> >   helper_argv = argv_split(GFP_KERNEL, cn.corename, NULL);
> >   call_usermodehelper_setup(helper_argv[0], ...);
> 
> Are you sure? See above.
> 

Yes, I'm sure. execve(NULL) from user space is safe, but
do_execve(NULL) from kernel space is not safe.

---------- patch start ----------
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -763,6 +763,9 @@ struct file *open_exec(const char *name)
 		.lookup_flags = LOOKUP_FOLLOW,
 	};
 
+	if (WARN_ON(!name))
+		return ERR_PTR(-EINVAL);
+
 	file = do_filp_open(AT_FDCWD, &tmp, &open_exec_flags);
 	if (IS_ERR(file))
 		goto out;
---------- patch end ----------

---------- dmesg start ----------
die[3924]: segfault at 0 ip 0804839c sp bf9d3c78 error 4 in die[8048000+1000]
------------[ cut here ]------------
WARNING: CPU: 1 PID: 3925 at fs/exec.c:766 open_exec+0xfd/0x110()
Modules linked in: ipv6 binfmt_misc
CPU: 1 PID: 3925 Comm: kworker/u4:0 Not tainted 3.11.0-10050-g3711d86-dirty #111
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 08/15/2008
 00000000 c0557e9e c068909b c0139756 c067af64 00000001 00000f55 c068909b
 000002fe c01da24d c01da24d 00000000 de754a80 df120b50 00000000 c013979b
 00000009 00000000 c01da24d de6626c0 c0158c68 df120b50 00000000 00000000
Call Trace:
 [<c0557e9e>] ? dump_stack+0x3e/0x50
 [<c0139756>] ? warn_slowpath_common+0x86/0xb0
 [<c01da24d>] ? open_exec+0xfd/0x110
 [<c01da24d>] ? open_exec+0xfd/0x110
 [<c013979b>] ? warn_slowpath_null+0x1b/0x20
 [<c01da24d>] ? open_exec+0xfd/0x110
 [<c0158c68>] ? prepare_creds+0x88/0xb0
 [<c01da98c>] ? do_execve+0x18c/0x560
 [<c014aa2c>] ? ____call_usermodehelper+0xbc/0xe0
 [<c05605f7>] ? ret_from_kernel_thread+0x1b/0x28
 [<c014aa50>] ? ____call_usermodehelper+0xe0/0xe0
---[ end trace 63bb92bc8d58b0c2 ]---
Core dump to | pipe failed
---------- dmesg end ----------

> Perhaps
> 
> 	--- x/kernel/kmod.c
> 	+++ x/kernel/kmod.c
> 	@@ -571,6 +571,9 @@ int call_usermodehelper_exec(struct subp
> 		DECLARE_COMPLETION_ONSTACK(done);
> 		int retval = 0;
> 	 
> 	+	if (!sub_info->path)
> 	+		return -EXXX;
> 	+
> 		helper_lock();
> 		if (!khelper_wq || usermodehelper_disabled) {
> 			retval = -EBUSY;
> 
> ?
> 

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