2003-11-17 20:00:41

by James Morris

[permalink] [raw]
Subject: [PATCH][RFC] Remove CLONE_FILES from init kernel thread creation

The patch below removes the CLONE_FILES flag from the kernel_thread() call
which starts init.

This is to prevent other kernel threads from sharing file descriptors
opened by init (try 'lsof /dev/initctl' on a 2.6 system :-).

The reason this patch is being proposed is so that usermode helper apps
launched via kernel threads (e.g. modprobe, hotplug) do not then inherit
any such file descriptors. This is not a problem in itself so far (other
than being messy), but it is a problem for SELinux, which will otherwise
need to grant access to /dev/initctl by modprobe and hotplug, a somewhat
undesirable scenario.

As far as I can tell, there is no reason why init needs to be spawned with
CLONE_FILES. Please let me know if there are any objections to the
change, which I would like to propose for 2.6.0+ as a cleanup.


- James
--
James Morris
<[email protected]>

diff -urN -X dontdiff linux-2.6.0-test9-mm3.orig/init/main.c linux-2.6.0-test9-mm3.w1/init/main.c
--- linux-2.6.0-test9-mm3.orig/init/main.c 2003-11-17 10:30:41.000000000 -0500
+++ linux-2.6.0-test9-mm3.w1/init/main.c 2003-11-17 14:27:07.000000000 -0500
@@ -375,7 +375,7 @@

static void rest_init(void)
{
- kernel_thread(init, NULL, CLONE_KERNEL);
+ kernel_thread(init, NULL, CLONE_FS | CLONE_SIGHAND);
unlock_kernel();
cpu_idle();
}


2003-11-17 20:49:32

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH][RFC] Remove CLONE_FILES from init kernel thread creation

James Morris <[email protected]> wrote:
>
> The patch below removes the CLONE_FILES flag from the kernel_thread() call
> which starts init.
>
> This is to prevent other kernel threads from sharing file descriptors
> opened by init (try 'lsof /dev/initctl' on a 2.6 system :-).
>
> The reason this patch is being proposed is so that usermode helper apps
> launched via kernel threads (e.g. modprobe, hotplug) do not then inherit
> any such file descriptors. This is not a problem in itself so far (other
> than being messy), but it is a problem for SELinux, which will otherwise
> need to grant access to /dev/initctl by modprobe and hotplug, a somewhat
> undesirable scenario.
>
> As far as I can tell, there is no reason why init needs to be spawned with
> CLONE_FILES. Please let me know if there are any objections to the
> change, which I would like to propose for 2.6.0+ as a cleanup.
>

No, I can't think of a reason why we'd need CLONE_FILES in there. I'll
toss it in and see what breaks.

I wonder why call_usermodehelper() uses CLONE_FILES...


2003-11-17 21:26:07

by Chris Wright

[permalink] [raw]
Subject: Re: [PATCH][RFC] Remove CLONE_FILES from init kernel thread creation

* Andrew Morton ([email protected]) wrote:
>
> I wonder why call_usermodehelper() uses CLONE_FILES...

Looks like 2.5 used to actually close all fd's before exec'ing the
usermodehelper. I wonder if it's just a result of Rusty's consolidation
and using CLONE_KERNEL?

thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net

2003-11-17 21:18:22

by James Morris

[permalink] [raw]
Subject: Re: [PATCH][RFC] Remove CLONE_FILES from init kernel thread creation

On Mon, 17 Nov 2003, Andrew Morton wrote:

> No, I can't think of a reason why we'd need CLONE_FILES in there. I'll
> toss it in and see what breaks.

Ok, also, for reference, Russell Coker discovered the issue and this fix
was suggested by Stephen Smalley.

> I wonder why call_usermodehelper() uses CLONE_FILES...

Because it's faster?


- James
--
James Morris
<[email protected]>