[PATCH] reboot: disable usermodehelper to prevent fs access

[PATCH] reboot: disable usermodehelper to prevent fs access

[Kay Sievers]

From: Kay Sievers <kay.sievers@vrfy.org>
Subject: [PATCH] reboot: disable usermodehelper to prevent fs access

In case CONFIG_UEVENT_HELPER_PATH is not set to "", which it
should be on every system, the kernel forks processes during
shutdown, which try to access the rootfs, even when the
binary does not exist. It causes exceptions and long delays in
the disk driver, which gets read requests at the time it tries
to shut down the disk.

This patch disables all kernel-forked processes during reboot to
allow a clean poweroff.

Cc: Tejun Heo <htejun@gmail.com>
Tested-By: Anton Guda <atu@dmeti.dp.ua>
Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---
 sys.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/sys.c b/kernel/sys.c
index af468ed..70c4c51 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -314,6 +314,7 @@ void kernel_restart_prepare(char *cmd)
 {
 	blocking_notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
 	system_state = SYSTEM_RESTART;
+	usermodehelper_disable();
 	device_shutdown();
 	sysdev_shutdown();
 	syscore_shutdown();
@@ -344,6 +345,7 @@ static void kernel_shutdown_prepare(enum system_states state)
 	blocking_notifier_call_chain(&reboot_notifier_list,
 		(state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
 	system_state = state;
+	usermodehelper_disable();
 	device_shutdown();
 }
 /**

Re: [PATCH] reboot: disable usermodehelper to prevent fs access

[Greg KH]

On Thu, May 05, 2011 at 01:32:05PM +0200, Kay Sievers wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: [PATCH] reboot: disable usermodehelper to prevent fs access
> 
> In case CONFIG_UEVENT_HELPER_PATH is not set to "", which it
> should be on every system, the kernel forks processes during
> shutdown, which try to access the rootfs, even when the
> binary does not exist. It causes exceptions and long delays in
> the disk driver, which gets read requests at the time it tries
> to shut down the disk.
> 
> This patch disables all kernel-forked processes during reboot to
> allow a clean poweroff.

Should this also be backported to the -stable kernels as people are
hitting this problem already today, right?

thanks,

greg k-h

Re: [PATCH] reboot: disable usermodehelper to prevent fs access

[Kay Sievers]

On Thu, May 5, 2011 at 22:27, Greg KH <greg@kroah.com> wrote:
> On Thu, May 05, 2011 at 01:32:05PM +0200, Kay Sievers wrote:
>> From: Kay Sievers <kay.sievers@vrfy.org>
>> Subject: [PATCH] reboot: disable usermodehelper to prevent fs access
>>
>> In case CONFIG_UEVENT_HELPER_PATH is not set to "", which it
>> should be on every system, the kernel forks processes during
>> shutdown, which try to access the rootfs, even when the
>> binary does not exist. It causes exceptions and long delays in
>> the disk driver, which gets read requests at the time it tries
>> to shut down the disk.
>>
>> This patch disables all kernel-forked processes during reboot to
>> allow a clean poweroff.
>
> Should this also be backported to the -stable kernels as people are
> hitting this problem already today, right?

If it survives fine, I guess it's nothing that will hurt us in
-stable. Suspend/hibernate does the same thing for similar reasons
since a while. Nothing really should use /sbin/hotplug anymore. It
just does not scale with what we do today with kernel devices.

Old udev SYSV init scripts or initramfs used to disable it at bootup.
But recent init systems just don't care anymore. So it just popped up
with systemd, where udev is a plain native service without "legacy
disablement".

So, it might be nice for recently released kernels, on the other hand
it will not cause any problems or data-loos, just a nasty delay at
shutdown, because of a kernel config option UEVENT_HELPER_PATH that is
set wrong for today's systems.

Kay

Re: [PATCH] reboot: disable usermodehelper to prevent fs access

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On Thu, 05 May 2011 13:32:05 +0200, Kay Sievers said:

> In case CONFIG_UEVENT_HELPER_PATH is not set to "", which it
> should be on every system

If it indeed should be that on every system, shouldn't it be listed
in feature-removal-schedule.txt?

Does anybody have a running list of "Stuff we set by default at one time, but
no longer recommend"?  I discovered that on my laptop, I have this sucker set
to /sbin/hotplug still (which explains why I get "filesystem busy" issues at
shutdown.  Just like the TCP_BIC issue yesterday, it's something that got set
aeons ago and 'make oldconfig' has been propagating.

But to clean things up, I'd have to know what stuff needed cleaning....



[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH] reboot: disable usermodehelper to prevent fs access

[Greg KH]

On Thu, May 05, 2011 at 05:24:25PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Thu, 05 May 2011 13:32:05 +0200, Kay Sievers said:
> 
> > In case CONFIG_UEVENT_HELPER_PATH is not set to "", which it
> > should be on every system
> 
> If it indeed should be that on every system, shouldn't it be listed
> in feature-removal-schedule.txt?

It's the default value, but distros, and people, can and do override it
for various reasons.  Why would it be added to
feature-removal-schedule.txt?

> Does anybody have a running list of "Stuff we set by default at one time, but
> no longer recommend"?

Look at the default values for different configurations options and why
they differ in your system is about the only way that I know of, sorry.

thanks,

greg k-h

Re: [PATCH] reboot: disable usermodehelper to prevent fs access

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

On Thu, 05 May 2011 14:34:58 PDT, Greg KH said:
> On Thu, May 05, 2011 at 05:24:25PM -0400, Valdis.Kletnieks@vt.edu wrote:
> > On Thu, 05 May 2011 13:32:05 +0200, Kay Sievers said:
> > > In case CONFIG_UEVENT_HELPER_PATH is not set to "", which it
> > > should be on every system
> >
> > If it indeed should be that on every system, shouldn't it be listed
> > in feature-removal-schedule.txt?
>
> It's the default value, but distros, and people, can and do override it
> for various reasons.  Why would it be added to
> feature-removal-schedule.txt?

Well, what Kay said was "it should be on *every* system", making it sound like
it's an option past its shelf life.  Certainly, "the default should be null for
the vast majority of systems" is a different scenario.

> > Does anybody have a running list of "Stuff we set by default at one time, but
> > no longer recommend"?

> Look at the default values for different configurations options and why
> they differ in your system is about the only way that I know of, sorry.

Hmm.. I suspect diffconfig will only get me part of the way there.  Maybe what
I *need* to do is find a 2.6.25-ish x86_64 defconfig, the current one, diff those, and
then see what changed (as opposed to truly new config flags), and then see
how many of those changes do/don't show up in *my* config as well..

Maybe that would be a good project for some #kernelnewbie to look at ;)
"In my copious free time" ;)


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH] reboot: disable usermodehelper to prevent fs access

[Kay Sievers]

On Fri, May 6, 2011 at 02:04,  <Valdis.Kletnieks@vt.edu> wrote:
> On Thu, 05 May 2011 14:34:58 PDT, Greg KH said:
>> On Thu, May 05, 2011 at 05:24:25PM -0400, Valdis.Kletnieks@vt.edu wrote:
>> > On Thu, 05 May 2011 13:32:05 +0200, Kay Sievers said:
>> > > In case CONFIG_UEVENT_HELPER_PATH is not set to "", which it
>> > > should be on every system
>> >
>> > If it indeed should be that on every system, shouldn't it be listed
>> > in feature-removal-schedule.txt?

Systems _very_ limited in scope might still want to use it, or it
theory it might be useful for debugging. On usual boxes it's nothing
but trouble, and on limited RAM boxes it's very a simple way to go
straight to OOM instead of booting up.

>> It's the default value, but distros, and people, can and do override it
>> for various reasons.  Why would it be added to
>> feature-removal-schedule.txt?

Yeah, nothing to remove really, someone could kill it from defconfig
though. I don't know what defconfig is really good for, and what are
the rules here, or why it's still in there.

> Well, what Kay said was "it should be on *every* system", making it sound like
> it's an option past its shelf life.  Certainly, "the default should be null for
> the vast majority of systems" is a different scenario.

The default is "" since quite a while. Just the archs defconfig might
not follow.

>> > Does anybody have a running list of "Stuff we set by default at one time, but
>> > no longer recommend"?

Nothing really at that scale. Running a binary from the kernel for
every event is really nothing we ever want to do these days.

>> Look at the default values for different configurations options and why
>> they differ in your system is about the only way that I know of, sorry.
>
> Hmm.. I suspect diffconfig will only get me part of the way there.  Maybe what
> I *need* to do is find a 2.6.25-ish x86_64 defconfig, the current one, diff those, and
> then see what changed (as opposed to truly new config flags), and then see
> how many of those changes do/don't show up in *my* config as well..
>
> Maybe that would be a good project for some #kernelnewbie to look at ;)
> "In my copious free time" ;)

Sure, sounds good. :)

Kay