2008-12-05 18:04:20

by Evgeniy Polyakov

[permalink] [raw]
Subject: Runaway loop with the current git.

Hi.

I've built current (aaad07763) git tree with config, which worked ok
with 2.6.27 (pressed enter several times when 'make oldconfig' asked)
and now system fails to boot.

Attached screen dump and config.

I'm not subscribed, please add to copy.

Thank you.

--
Evgeniy Polyakov


Attachments:
(No filename) (284.00 B)
runaway_loop.png (15.70 kB)
.config (31.15 kB)
Download all attachments

2008-12-05 18:16:28

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Fri, 5 Dec 2008 21:03:50 +0300
Evgeniy Polyakov <[email protected]> wrote:

> Hi.
>
> I've built current (aaad07763) git tree with config, which worked ok
> with 2.6.27 (pressed enter several times when 'make oldconfig' asked)
> and now system fails to boot.
>
> Attached screen dump and config.
>
> I'm not subscribed, please add to copy.

Your modprobe is for some reason triggering a load of /dev/ttyS* (ie the
serial 8250 stuff) from within the module loading scripts. You'll need to
debug the user space and work out why the loop is happening then it
should be fairly easy to fix.

Alan

2008-12-05 18:32:28

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Fri, Dec 5, 2008 at 19:16, Alan Cox <[email protected]> wrote:
> On Fri, 5 Dec 2008 21:03:50 +0300
> Evgeniy Polyakov <[email protected]> wrote:

>> I've built current (aaad07763) git tree with config, which worked ok
>> with 2.6.27 (pressed enter several times when 'make oldconfig' asked)
>> and now system fails to boot.
>>
>> Attached screen dump and config.
>>
>> I'm not subscribed, please add to copy.
>
> Your modprobe is for some reason triggering a load of /dev/ttyS* (ie the
> serial 8250 stuff) from within the module loading scripts. You'll need to
> debug the user space and work out why the loop is happening then it
> should be fairly easy to fix.

Why do you think it's serial? 5-1 looks more like /dev/console.

This also seems to happen only in initramfs for people who see this.
People say that compiling-in some modules (unfortunately they don't
remember which) makes it work again.

Can this be some init order problem, that the /dev/console is not
initialized but, we fork a process which tries to access it?

Kay

2008-12-05 19:28:15

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Fri, Dec 05, 2008 at 07:32:15PM +0100, Kay Sievers ([email protected]) wrote:
> > Your modprobe is for some reason triggering a load of /dev/ttyS* (ie the
> > serial 8250 stuff) from within the module loading scripts. You'll need to
> > debug the user space and work out why the loop is happening then it
> > should be fairly easy to fix.
>
> Why do you think it's serial? 5-1 looks more like /dev/console.
>
> This also seems to happen only in initramfs for people who see this.
> People say that compiling-in some modules (unfortunately they don't
> remember which) makes it work again.
>
> Can this be some init order problem, that the /dev/console is not
> initialized but, we fork a process which tries to access it?

Getting that the same userspace runs ok in 2.6.27 and config changes do
not show immediate reason, I would blame some internal changes in some
kernel subsystem. Why at first it infinitely loops in tty/console loading?

I can not run bisect, since reboot requires vnc and I'm far from fast
channel. Build of the fresh tree with existing config takes almost
one hour, but it should not be a major problem.

I can do bisection after the weekend, but can test some changes before
machine froze with unsuccessful patch.

--
Evgeniy Polyakov

2008-12-05 19:34:38

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> I can not run bisect, since reboot requires vnc and I'm far from fast
> channel. Build of the fresh tree with existing config takes almost
> one hour, but it should not be a major problem.
>
> I can do bisection after the weekend, but can test some changes before
> machine froze with unsuccessful patch.

I really have no idea what you are seeing so short of a bisect I've
nothing to suggest.

2008-12-05 21:12:39

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Fri, Dec 05, 2008 at 07:34:32PM +0000, Alan Cox ([email protected]) wrote:
> > I can not run bisect, since reboot requires vnc and I'm far from fast
> > channel. Build of the fresh tree with existing config takes almost
> > one hour, but it should not be a major problem.
> >
> > I can do bisection after the weekend, but can test some changes before
> > machine froze with unsuccessful patch.
>
> I really have no idea what you are seeing so short of a bisect I've
> nothing to suggest.

But if things would be in userspace, then it should be fired long ago
with .27 kernel, no?

And what's with tty drivers? If kernel freezes there even if suddenly
faulty userspace started to load them, where this can happen? Apparently
it is not sleeping userspace since console does not respond to input.

--
Evgeniy Polyakov

2008-12-05 21:17:19

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Fri, Dec 5, 2008 at 22:12, Evgeniy Polyakov <[email protected]> wrote:
> On Fri, Dec 05, 2008 at 07:34:32PM +0000, Alan Cox ([email protected]) wrote:
>> > I can not run bisect, since reboot requires vnc and I'm far from fast
>> > channel. Build of the fresh tree with existing config takes almost
>> > one hour, but it should not be a major problem.
>> >
>> > I can do bisection after the weekend, but can test some changes before
>> > machine froze with unsuccessful patch.
>>
>> I really have no idea what you are seeing so short of a bisect I've
>> nothing to suggest.
>
> But if things would be in userspace, then it should be fired long ago
> with .27 kernel, no?
>
> And what's with tty drivers? If kernel freezes there even if suddenly
> faulty userspace started to load them, where this can happen? Apparently
> it is not sleeping userspace since console does not respond to input.

5-1 dev_t request looks like something in initramfs accesses
/dev/console, but the driver for it is not properly
initialized/registered that time, and the kernel module loader tries
to load a module? The forked process might also try to access
/dev/console again, and hence the loop?

Kay

2008-12-05 21:24:38

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Fri, Dec 05, 2008 at 10:17:01PM +0100, Kay Sievers ([email protected]) wrote:
> > And what's with tty drivers? If kernel freezes there even if suddenly
> > faulty userspace started to load them, where this can happen? Apparently
> > it is not sleeping userspace since console does not respond to input.
>
> 5-1 dev_t request looks like something in initramfs accesses
> /dev/console, but the driver for it is not properly
> initialized/registered that time, and the kernel module loader tries
> to load a module? The forked process might also try to access
> /dev/console again, and hence the loop?

But why system freezes? Is this init linking/__init calling order
changes? I have all tty/8250 compiled in btw.

$ egrep -i "console|tty|8250" /tmp/.config
# CONFIG_NETFILTER_XT_MATCH_PKTTYPE is not set
CONFIG_NETCONSOLE=m
# CONFIG_NETCONSOLE_DYNAMIC is not set
CONFIG_CONSOLE_TRANSLATIONS=y
CONFIG_VT_CONSOLE=y
CONFIG_HW_CONSOLE=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_8250_PCI=y
CONFIG_SERIAL_8250_PNP=y
CONFIG_SERIAL_8250_NR_UARTS=16
CONFIG_SERIAL_8250_RUNTIME_UARTS=4
# CONFIG_SERIAL_8250_EXTENDED is not set
# Non-8250 serial port support
CONFIG_SERIAL_CORE_CONSOLE=y
# Console display driver support
CONFIG_VGA_CONSOLE=y
CONFIG_DUMMY_CONSOLE=y

--
Evgeniy Polyakov

2008-12-06 00:29:19

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, 6 Dec 2008 00:12:23 +0300
Evgeniy Polyakov <[email protected]> wrote:

> On Fri, Dec 05, 2008 at 07:34:32PM +0000, Alan Cox ([email protected]) wrote:
> > > I can not run bisect, since reboot requires vnc and I'm far from fast
> > > channel. Build of the fresh tree with existing config takes almost
> > > one hour, but it should not be a major problem.
> > >
> > > I can do bisection after the weekend, but can test some changes before
> > > machine froze with unsuccessful patch.
> >
> > I really have no idea what you are seeing so short of a bisect I've
> > nothing to suggest.
>
> But if things would be in userspace, then it should be fired long ago
> with .27 kernel, no?

I have no idea. A bisection will hopefully provide answers

2008-12-06 02:10:45

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Fri, Dec 5, 2008 at 22:24, Evgeniy Polyakov <[email protected]> wrote:
> On Fri, Dec 05, 2008 at 10:17:01PM +0100, Kay Sievers ([email protected]) wrote:
>> > And what's with tty drivers? If kernel freezes there even if suddenly
>> > faulty userspace started to load them, where this can happen? Apparently
>> > it is not sleeping userspace since console does not respond to input.
>>
>> 5-1 dev_t request looks like something in initramfs accesses
>> /dev/console, but the driver for it is not properly
>> initialized/registered that time, and the kernel module loader tries
>> to load a module? The forked process might also try to access
>> /dev/console again, and hence the loop?
>
> But why system freezes? Is this init linking/__init calling order
> changes?

Can you add something like:
+++ b/kernel/kmod.c
@@ -108,6 +108,7 @@ int request_module(const char *fmt, ...)
return -ENOMEM;
}

+ printk("XXX call modprobe %s %s[%u]\n", module_name,
current->comm, task_pid_nr(current));

It may show which process is looking for /dev/console and causes
modprobe to run, and maybe we get an idea what's going on. It may at
least show if it's a /dev/console problem.

Thanks,
Kay

2008-12-06 16:09:37

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, Dec 06, 2008 at 03:10:33AM +0100, Kay Sievers ([email protected]) wrote:
> Can you add something like:
> +++ b/kernel/kmod.c
> @@ -108,6 +108,7 @@ int request_module(const char *fmt, ...)
> return -ENOMEM;
> }
>
> + printk("XXX call modprobe %s %s[%u]\n", module_name,
> current->comm, task_pid_nr(current));
>
> It may show which process is looking for /dev/console and causes
> modprobe to run, and maybe we get an idea what's going on. It may at
> least show if it's a /dev/console problem.

Its a modprobe with different pids tries to load char-major-5 and
char-major-5-1 in the infinite loop.

--
Evgeniy Polyakov

2008-12-06 16:23:09

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, Dec 6, 2008 at 17:09, Evgeniy Polyakov <[email protected]> wrote:
> On Sat, Dec 06, 2008 at 03:10:33AM +0100, Kay Sievers ([email protected]) wrote:
>> Can you add something like:
>> +++ b/kernel/kmod.c
>> @@ -108,6 +108,7 @@ int request_module(const char *fmt, ...)
>> return -ENOMEM;
>> }
>>
>> + printk("XXX call modprobe %s %s[%u]\n", module_name,
>> current->comm, task_pid_nr(current));
>>
>> It may show which process is looking for /dev/console and causes
>> modprobe to run, and maybe we get an idea what's going on. It may at
>> least show if it's a /dev/console problem.
>
> Its a modprobe with different pids tries to load char-major-5 and
> char-major-5-1 in the infinite loop.

So the loop is probably a modprobe itself that tries to access
/dev/console. Is there a different argument for the very first
modprobe which is called? Which may be the one that triggers the loop.

Kay

2008-12-06 16:56:46

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, Dec 06, 2008 at 05:16:06PM +0100, Kay Sievers ([email protected]) wrote:
> So the loop is probably a modprobe itself that tries to access
> /dev/console. Is there a different argument for the very first
> modprobe which is called? Which may be the one that triggers the loop.

Hard to tell, I did not see anything but modprobe before and after
runaway loop message, but it could be missed though, I will tell
for sure only this Monday.

--
Evgeniy Polyakov

2008-12-06 18:12:01

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, Dec 6, 2008 at 17:56, Evgeniy Polyakov <[email protected]> wrote:
> On Sat, Dec 06, 2008 at 05:16:06PM +0100, Kay Sievers ([email protected]) wrote:
>> So the loop is probably a modprobe itself that tries to access
>> /dev/console. Is there a different argument for the very first
>> modprobe which is called? Which may be the one that triggers the loop.
>
> Hard to tell, I did not see anything but modprobe before and after
> runaway loop message, but it could be missed though, I will tell
> for sure only this Monday.

Sounds good.

It seems the /dev/console driver is registered only after all the pci,
video, acpi, ... drivers, so it's not surprising, that if any of these
driver in these subsystems calls request_module(), or a process in
initramfs tries to access the "dead" /dev/console node, things will go
wrong.

Thanks,
Kay

2008-12-06 19:32:29

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, 2008-12-06 at 19:11 +0100, Kay Sievers wrote:
> On Sat, Dec 6, 2008 at 17:56, Evgeniy Polyakov <[email protected]> wrote:
> > On Sat, Dec 06, 2008 at 05:16:06PM +0100, Kay Sievers ([email protected]) wrote:
> >> So the loop is probably a modprobe itself that tries to access
> >> /dev/console. Is there a different argument for the very first
> >> modprobe which is called? Which may be the one that triggers the loop.
> >
> > Hard to tell, I did not see anything but modprobe before and after
> > runaway loop message, but it could be missed though, I will tell
> > for sure only this Monday.
>
> Sounds good.
>
> It seems the /dev/console driver is registered only after all the pci,
> video, acpi, ... drivers, so it's not surprising, that if any of these
> driver in these subsystems calls request_module(), or a process in
> initramfs tries to access the "dead" /dev/console node, things will go
> wrong.

I don't know if it may have any bad side-effects. It moves the tty
registration earlier, before we do pci, framebuffer, video, acpi
registration.

It boots fine here with and without initramfs.

Maybe it makes the "dead" /dev/console in your initramfs working, then
we at least know the problem.

Thanks,
Kay



diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 1412a8d..72bc98f 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -3115,26 +3115,14 @@ void __init console_init(void)
}
}

-static int __init tty_class_init(void)
+static struct cdev tty_cdev, console_cdev;
+
+static int __init tty_init(void)
{
tty_class = class_create(THIS_MODULE, "tty");
if (IS_ERR(tty_class))
return PTR_ERR(tty_class);
- return 0;
-}
-
-postcore_initcall(tty_class_init);
-
-/* 3/2004 jmc: why do these devices exist? */

-static struct cdev tty_cdev, console_cdev;
-
-/*
- * Ok, now we can initialize the rest of the tty devices and can count
- * on memory allocations, interrupts etc..
- */
-static int __init tty_init(void)
-{
cdev_init(&tty_cdev, &tty_fops);
if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
@@ -3154,4 +3142,4 @@ static int __init tty_init(void)
#endif
return 0;
}
-module_init(tty_init);
+postcore_initcall(tty_init);

2008-12-06 20:26:32

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, Dec 06, 2008 at 08:32:06PM +0100, Kay Sievers ([email protected]) wrote:
> I don't know if it may have any bad side-effects. It moves the tty
> registration earlier, before we do pci, framebuffer, video, acpi
> registration.
>
> It boots fine here with and without initramfs.
>
> Maybe it makes the "dead" /dev/console in your initramfs working, then
> we at least know the problem.

I will try this patch this Monday and report back the results.

Thank you.

--
Evgeniy Polyakov

2008-12-07 03:56:20

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sat, Dec 6, 2008 at 21:26, Evgeniy Polyakov <[email protected]> wrote:
> On Sat, Dec 06, 2008 at 08:32:06PM +0100, Kay Sievers ([email protected]) wrote:
>> I don't know if it may have any bad side-effects. It moves the tty
>> registration earlier, before we do pci, framebuffer, video, acpi
>> registration.
>>
>> It boots fine here with and without initramfs.
>>
>> Maybe it makes the "dead" /dev/console in your initramfs working, then
>> we at least know the problem.
>
> I will try this patch this Monday and report back the results.

I was able to reproduce it with the .config you attached, and running it in kvm.

It is caused by:
"modprobe cryptomgr" called from swapper[1]

This modprobe process does try to log an error, accesses /dev/console,
which is not initialized in the kernel at that time, and the kernel
module loader tries the load a module to support dev_t 5:1, which
again runs modprobe, and ...

Setting CONFIG_CRYPTO_MANAGER=y makes it disapper. The patch I sent
seems to fix it.

The bug is handled here: http://bugzilla.kernel.org/show_bug.cgi?id=12153

Thanks,
Kay

2008-12-07 04:31:28

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 04:56:04AM +0100, Kay Sievers ([email protected]) wrote:
> I was able to reproduce it with the .config you attached, and running it in kvm.
>
> It is caused by:
> "modprobe cryptomgr" called from swapper[1]
>
> This modprobe process does try to log an error, accesses /dev/console,
> which is not initialized in the kernel at that time, and the kernel
> module loader tries the load a module to support dev_t 5:1, which
> again runs modprobe, and ...
>
> Setting CONFIG_CRYPTO_MANAGER=y makes it disapper. The patch I sent
> seems to fix it.

Great to know it works. I will test it when get access to the machine
after the weekend.

> The bug is handled here: http://bugzilla.kernel.org/show_bug.cgi?id=12153

There is a good point in the thread, that hotplug should not call
recursion, but while this may be very valid requirement, it is _very_
subtle, and something more solid should be done. Just an opinion.

Initialization reordering is likely also not what we want, but simple
dumb console (or just dummy device which will accept data and will be
replaced later) sounds like a way to go, and your patch seems to allow
that dumb class registration first.
I actually used to believe that it works that way already...

--
Evgeniy Polyakov

2008-12-07 11:23:42

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> This modprobe process does try to log an error, accesses /dev/console,
> which is not initialized in the kernel at that time, and the kernel
> module loader tries the load a module to support dev_t 5:1, which
> again runs modprobe, and ...

So we have a buggy modprobe...

>
> Setting CONFIG_CRYPTO_MANAGER=y makes it disapper. The patch I sent
> seems to fix it.
>
> The bug is handled here: http://bugzilla.kernel.org/show_bug.cgi?id=12153

We cannot go re-ordering random chunks of kernel init with unpredictable
effects including possibly making other stuff less reliable (because you
set up the console device before the console driver is loaded on a PCI
bus device). And we certainly can't do it this close to a release.

Alan

2008-12-07 11:45:30

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 11:23:35AM +0000, Alan Cox ([email protected]) wrote:
> > This modprobe process does try to log an error, accesses /dev/console,
> > which is not initialized in the kernel at that time, and the kernel
> > module loader tries the load a module to support dev_t 5:1, which
> > again runs modprobe, and ...
>
> So we have a buggy modprobe...

Which nevertheless should not break boot process...

> > Setting CONFIG_CRYPTO_MANAGER=y makes it disapper. The patch I sent
> > seems to fix it.
> >
> > The bug is handled here: http://bugzilla.kernel.org/show_bug.cgi?id=12153
>
> We cannot go re-ordering random chunks of kernel init with unpredictable
> effects including possibly making other stuff less reliable (because you
> set up the console device before the console driver is loaded on a PCI
> bus device). And we certainly can't do it this close to a release.

Alan, really, do you believe that relying on userspace to be always
correct is the way to go? Requrement for the userspace to always have
enough buffer available when doing some reading is essetually the same.
We want userspace to be non-recursive, but if this does not happen, we
should not hung. Detect recursion, do not allow more than 1-2-3
simultaneous modprobes, whatever, but do not say, that kernel behaves
that bad just because userspace is allowed to do that.

And what's the argument of being close to a release? Do you propose to
hide the head into the sand and point a finger to anyone else saying its
not a kernel's problem? If prerelease has a bug, it should be fixed, and
not hidden under the cover.

What about storing a small stack of recently requested device ids, and
if new request is about to ask one from that stack, return error? I can
cook up the patch tomorrow.

--
Evgeniy Polyakov

2008-12-07 11:58:20

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> > So we have a buggy modprobe...
>
> Which nevertheless should not break boot process...

Now you are talking complete crap. If init crashes what happens. If I
have the wrong root fs listed what happens ?

Gee it breaks. Early userspace actually does have to be correct.

> should not hung. Detect recursion, do not allow more than 1-2-3
> simultaneous modprobes, whatever, but do not say, that kernel behaves
> that bad just because userspace is allowed to do that.

Please read the code: that is what we do. That is *WHY* you got the
runaway modprobe message. It was caught but despite that the broken
initrd failed to recover. We've had that feature for years.

> And what's the argument of being close to a release? Do you propose to
> hide the head into the sand and point a finger to anyone else saying its
> not a kernel's problem? If prerelease has a bug, it should be fixed, and
> not hidden under the cover.

Its an incredibly obscure corner case. It is not appropriate to totally
trash years of kernel testing by panic re-ordering some init functions
just before a release. You will undoubtedly harm more people than you'll
please.

> What about storing a small stack of recently requested device ids, and
> if new request is about to ask one from that stack, return error? I can
> cook up the patch tomorrow.

That would be an interesting experiment for 2.6.29 however we do requests
in lots of different formats so it might be a bit more complex. You could
store the last few strings. However the existing runaway code caught it
and will have resulted in the -ENXIO path occuring anyway so I don't see
what your "change" will achieve.

Alan

2008-12-07 13:10:23

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 11:58:20AM +0000, Alan Cox ([email protected]) wrote:
> > Which nevertheless should not break boot process...
>
> Now you are talking complete crap. If init crashes what happens. If I
> have the wrong root fs listed what happens ?
>
> Gee it breaks. Early userspace actually does have to be correct.

Heh, if userspace writes more than kernel's buffer size, what happens?
We suddenly stopped to trust userspace? What if modprobe itself will do
that? Do not differentiate bugs by the nature of not-bad and bad-enough:
we do not trust userspace, even init, and it looking at the very end of
the mail you will find again why this loading crap is a kernel problem.

> > should not hung. Detect recursion, do not allow more than 1-2-3
> > simultaneous modprobes, whatever, but do not say, that kernel behaves
> > that bad just because userspace is allowed to do that.
>
> Please read the code: that is what we do. That is *WHY* you got the
> runaway modprobe message. It was caught but despite that the broken
> initrd failed to recover. We've had that feature for years.

Apparently just reading the code does not magically fix kernel problems.
What's the cosmic rays should initird check when it is asked by kernel
to load the module? It tries to load it, which in turn tries to load it
again. Since initird does not use libastral yet, it is up to kernel to
decide what to load and when to ask userspace about that.

> > And what's the argument of being close to a release? Do you propose to
> > hide the head into the sand and point a finger to anyone else saying its
> > not a kernel's problem? If prerelease has a bug, it should be fixed, and
> > not hidden under the cover.
>
> Its an incredibly obscure corner case. It is not appropriate to totally
> trash years of kernel testing by panic re-ordering some init functions
> just before a release. You will undoubtedly harm more people than you'll
> please.

Registering purely software class device not being backed by real
hardware since appropriate buses are not yet available? Apparently I
know how to fix this problem: I will write a simple console driver
called 'prayer', which will just sacrifice a little CPU's heat to the
Raa-god when anyone tries to write something to console. And this driver
will later be replaced with real console. Ooops, doesn't above patch
from Kay do the same?

> > What about storing a small stack of recently requested device ids, and
> > if new request is about to ask one from that stack, return error? I can
> > cook up the patch tomorrow.
>
> That would be an interesting experiment for 2.6.29 however we do requests
> in lots of different formats so it might be a bit more complex. You could
> store the last few strings. However the existing runaway code caught it
> and will have resulted in the -ENXIO path occuring anyway so I don't see
> what your "change" will achieve.

It will not infinitely try to load module, which tries to load itself
again via obscure kernel pathes, return error and continue.

I remind again: all console/tty stuff in my setup is built-in, so there
should be no module request at all, but since earlier registered code
does not know that, kernel believes that console is somewhere in the
module. Having dumb device registered first is a fix for this problem.

--
Evgeniy Polyakov

2008-12-07 14:02:58

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 12:23, Alan Cox <[email protected]> wrote:
>> This modprobe process does try to log an error, accesses /dev/console,
>> which is not initialized in the kernel at that time, and the kernel
>> module loader tries the load a module to support dev_t 5:1, which
>> again runs modprobe, and ...
>
> So we have a buggy modprobe...

No, we have not. It is fine for modprobe doing that, as it is for any
other binary too. It's also the usual glibc behavior for syslog(). Any
userspace binary can access /dev/console any time. The kernel is not
supposed to call modprobe to make /dev/console availabe.

>> Setting CONFIG_CRYPTO_MANAGER=y makes it disapper. The patch I sent
>> seems to fix it.
>>
>> The bug is handled here: http://bugzilla.kernel.org/show_bug.cgi?id=12153
>
> We cannot go re-ordering random chunks of kernel init with unpredictable
> effects including possibly making other stuff less reliable (because you
> set up the console device before the console driver is loaded on a PCI
> bus device).

We are not reordering, we provide a registered /dev/console driver
core device, not touching any driver, just to prevent the kernel
module loader from going crazy.

> And we certainly can't do it this close to a release.

We can do the real fix any time it is the proper time. It is still an
obvious kernel bug which needs to be fixed, and not some obscure
userspace rule, we suddenly need to establish to work around some dumb
module loader/console logic.

Thanks,
Kay

2008-12-07 14:49:52

by Herbert Xu

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

Alan Cox <[email protected]> wrote:
>> This modprobe process does try to log an error, accesses /dev/console,
>> which is not initialized in the kernel at that time, and the kernel
>> module loader tries the load a module to support dev_t 5:1, which
>> again runs modprobe, and ...
>
> So we have a buggy modprobe...

FWIW the crypto layer is doing what is intended. The algorithm
testing infrastructure in the cryptomgr module is optional and
only used when algorithms are registered. That's why it sits
in its own module which is loaded by the algorithm registration
code which still functions if the module is absent or fails to
load.

In any case the loop itself does not involve any crypto components
so I don't think making changes in the crypto layer is going to
make this go away forever as anyone calling request_module early
enough will get into this loop.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-12-07 15:08:59

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> No, we have not. It is fine for modprobe doing that, as it is for any
> other binary too. It's also the usual glibc behavior for syslog(). Any
> userspace binary can access /dev/console any time. The kernel is not
> supposed to call modprobe to make /dev/console availabe.

Yes it is. The hotplug interface is designed to handle dynamic loading of
devices including the console. Likewise your hard disk interfaces you can
access at any time - guess what, those need loading too and if you put
your modprobe on a disk you've not got a driver loaded for that doesn't
work either.

> We are not reordering, we provide a registered /dev/console driver
> core device, not touching any driver, just to prevent the kernel
> module loader from going crazy.

You are reordering things.

The kernel module loader isn't going crazy either. The kernel module
loader is doings its job. The userspace then goes silly and the kernel
module loader correctly traps the problem, logs an error and tries to
continue things, but the initrd is it seems too broken to handle that
anyway.

Note that last small rather important detail - the kernel does detect
runaway modprobe loops already.

Alan

2008-12-07 15:16:20

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> In any case the loop itself does not involve any crypto components
> so I don't think making changes in the crypto layer is going to
> make this go away forever as anyone calling request_module early
> enough will get into this loop.

Thanks - closed as WONTFIX as this is ultimately a user space bug caused
by picking particularly odd combinations of configuration options (serial
console, crypto testing) combined with a problem with someones specific
initrd

Alan

2008-12-07 15:55:32

by Herbert Xu

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 10:49:27PM +0800, Herbert Xu wrote:
>
> In any case the loop itself does not involve any crypto components
> so I don't think making changes in the crypto layer is going to
> make this go away forever as anyone calling request_module early
> enough will get into this loop.

Having said that I think this patch should remove this particular
trigger. Unfortunately I couldn't find a more succinct way of
expressing this relationship: if ALGAPI is built-in because it's
selected by an algorithm, and CRYPTO_MANAGER is enabled then we
require CRYPTO_MANAGER to be built-in as well.

Evgeniy, please let me know whether this fixes the problem.

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 6593b5a..3f88a52 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -31,35 +31,63 @@ config CRYPTO_FIPS

config CRYPTO_ALGAPI
tristate
+ select CRYPTO_ALGAPI2
help
This option provides the API for cryptographic algorithms.

+config CRYPTO_ALGAPI2
+ tristate
+
config CRYPTO_AEAD
tristate
+ select CRYPTO_AEAD2
select CRYPTO_ALGAPI

+config CRYPTO_AEAD2
+ tristate
+ select CRYPTO_ALGAPI2
+
config CRYPTO_BLKCIPHER
tristate
+ select CRYPTO_BLKCIPHER2
select CRYPTO_ALGAPI
- select CRYPTO_RNG
+
+config CRYPTO_BLKCIPHER2
+ tristate
+ select CRYPTO_ALGAPI2
+ select CRYPTO_RNG2

config CRYPTO_HASH
tristate
+ select CRYPTO_HASH2
select CRYPTO_ALGAPI

+config CRYPTO_HASH2
+ tristate
+ select CRYPTO_ALGAPI2
+
config CRYPTO_RNG
tristate
+ select CRYPTO_RNG2
select CRYPTO_ALGAPI

+config CRYPTO_RNG2
+ tristate
+ select CRYPTO_ALGAPI2
+
config CRYPTO_MANAGER
tristate "Cryptographic algorithm manager"
- select CRYPTO_AEAD
- select CRYPTO_HASH
- select CRYPTO_BLKCIPHER
+ select CRYPTO_MANAGER2
help
Create default cryptographic template instantiations such as
cbc(aes).

+config CRYPTO_MANAGER2
+ def_tristate CRYPTO_MANAGER || (CRYPTO_MANAGER!=n && CRYPTO_ALGAPI=y)
+ select CRYPTO_AEAD2
+ select CRYPTO_HASH2
+ select CRYPTO_BLKCIPHER2
+
config CRYPTO_GF128MUL
tristate "GF(2^128) multiplication functions (EXPERIMENTAL)"
depends on EXPERIMENTAL
diff --git a/crypto/Makefile b/crypto/Makefile
index 0e93f32..73e7c30 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -9,25 +9,25 @@ obj-$(CONFIG_CRYPTO_FIPS) += fips.o

crypto_algapi-$(CONFIG_PROC_FS) += proc.o
crypto_algapi-objs := algapi.o scatterwalk.o $(crypto_algapi-y)
-obj-$(CONFIG_CRYPTO_ALGAPI) += crypto_algapi.o
+obj-$(CONFIG_CRYPTO_ALGAPI2) += crypto_algapi.o

-obj-$(CONFIG_CRYPTO_AEAD) += aead.o
+obj-$(CONFIG_CRYPTO_AEAD2) += aead.o

crypto_blkcipher-objs := ablkcipher.o
crypto_blkcipher-objs += blkcipher.o
-obj-$(CONFIG_CRYPTO_BLKCIPHER) += crypto_blkcipher.o
-obj-$(CONFIG_CRYPTO_BLKCIPHER) += chainiv.o
-obj-$(CONFIG_CRYPTO_BLKCIPHER) += eseqiv.o
+obj-$(CONFIG_CRYPTO_BLKCIPHER2) += crypto_blkcipher.o
+obj-$(CONFIG_CRYPTO_BLKCIPHER2) += chainiv.o
+obj-$(CONFIG_CRYPTO_BLKCIPHER2) += eseqiv.o
obj-$(CONFIG_CRYPTO_SEQIV) += seqiv.o

crypto_hash-objs := hash.o
crypto_hash-objs += ahash.o
crypto_hash-objs += shash.o
-obj-$(CONFIG_CRYPTO_HASH) += crypto_hash.o
+obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o

cryptomgr-objs := algboss.o testmgr.o

-obj-$(CONFIG_CRYPTO_MANAGER) += cryptomgr.o
+obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
obj-$(CONFIG_CRYPTO_HMAC) += hmac.o
obj-$(CONFIG_CRYPTO_XCBC) += xcbc.o
obj-$(CONFIG_CRYPTO_NULL) += crypto_null.o

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-12-07 16:03:29

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 16:55, Herbert Xu <[email protected]> wrote:
> On Sun, Dec 07, 2008 at 10:49:27PM +0800, Herbert Xu wrote:
>>
>> In any case the loop itself does not involve any crypto components
>> so I don't think making changes in the crypto layer is going to
>> make this go away forever as anyone calling request_module early
>> enough will get into this loop.
>
> Having said that I think this patch should remove this particular
> trigger. Unfortunately I couldn't find a more succinct way of
> expressing this relationship: if ALGAPI is built-in because it's
> selected by an algorithm, and CRYPTO_MANAGER is enabled then we
> require CRYPTO_MANAGER to be built-in as well.
>
> Evgeniy, please let me know whether this fixes the problem.

Even when this works around it, the problem that the kernel triggers
module loading on /dev/console access stays and needs to be fixed
instead.

It's a pure kernel bug, regardless of how many times Alan likes to
establish non-sensical hotplug rules to prevent this. The next
innocent driver change, like yours is, will run into the same problem.

Thanks,
Kay

2008-12-07 16:11:29

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> Even when this works around it, the problem that the kernel triggers
> module loading on /dev/console access stays and needs to be fixed
> instead.

It isn't a problem. It is trying to have hotplug load a suitable driver.
This is what is supposed to happen.

Alan

2008-12-07 16:21:19

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 17:09, Alan Cox <[email protected]> wrote:
>> Even when this works around it, the problem that the kernel triggers
>> module loading on /dev/console access stays and needs to be fixed
>> instead.
>
> It isn't a problem. It is trying to have hotplug load a suitable driver.
> This is what is supposed to happen.

No, it's not. 5:1 is _in_ the kernel, and must not be tried to be
loaded by the kernel. We need to make /dev/console access return
-ENODEV if not available, not try to load a module for it.

It is a problem. It's broken behavior, and it breaks shipped products
with any innocent future driver change like Herbert's.

Please start to think about the problem, and don't repeat things that
do not make sense, and never made any sense.

Thanks,
Kay

2008-12-07 16:32:07

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 04:09:21PM +0000, Alan Cox ([email protected]) wrote:
> > Even when this works around it, the problem that the kernel triggers
> > module loading on /dev/console access stays and needs to be fixed
> > instead.
>
> It isn't a problem. It is trying to have hotplug load a suitable driver.
> This is what is supposed to happen.

Wrong. First at least because tty/console stuff is built in.
But we alredy got, that you decided that Debian's initramfs-tools (0.92e)
are allowed not to boot with some kernel configs.

--
Evgeniy Polyakov

2008-12-07 16:33:34

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 11:55:07PM +0800, Herbert Xu ([email protected]) wrote:
> Having said that I think this patch should remove this particular
> trigger. Unfortunately I couldn't find a more succinct way of
> expressing this relationship: if ALGAPI is built-in because it's
> selected by an algorithm, and CRYPTO_MANAGER is enabled then we
> require CRYPTO_MANAGER to be built-in as well.
>
> Evgeniy, please let me know whether this fixes the problem.

Holy shit... This looks 'excellent' :)

I will try it tomorrow and report back the results.

--
Evgeniy Polyakov

2008-12-07 16:57:46

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> > It isn't a problem. It is trying to have hotplug load a suitable driver.
> > This is what is supposed to happen.
>
> No, it's not. 5:1 is _in_ the kernel, and must not be tried to be
> loaded by the kernel. We need to make /dev/console access return
> -ENODEV if not available, not try to load a module for it.

Hello earth calling, wake up.

The userspace opens major 5 minor 1. The kernel has no driver mapped to
that so the kernel asks user space to load a module of its choice for
major 5 minor 1. What user space does with that is and has always been a
problem for user space.

User space is quite at liberty to go .. 5,1 and I have a serial console
I want to load 8250_pci please.

What it must not do is try and re-open it again and again and again.

You may not assume that a given device load mapping is solely used to
create the most basic device node involved. It isn't that simple. In many
cases a device node translates to a series of module loads of otherwise
apparently unrelated devices.

2008-12-07 17:01:16

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> Wrong. First at least because tty/console stuff is built in.

Are you two people or just two names and email addresses for
the same ?

/dev/console is a logical mapping to a device which may well be
different, loaded after PCI is initialised and dependant on PCI.

> But we alredy got, that you decided that Debian's initramfs-tools (0.92e)
> are allowed not to boot with some kernel configs.

Yes. They won't boot if you don't include any disk drivers. They won't
boot if you don't include any file systems. They wont boot in a million
other cases. That is the user space not the kernel at fault.

The kernel is doing precisely what it is supposed to. It is even logging
the user space bug and stopping trying to get stuck in a loop loading a
module in order to attempt to cope with the user space bug.

If you want to complain then file a debian bug, go fix the user space.

Alan

2008-12-07 17:04:00

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 04:57:40PM +0000, Alan Cox ([email protected]) wrote:
> The userspace opens major 5 minor 1. The kernel has no driver mapped to
> that so the kernel asks user space to load a module of its choice for

It has. It really has it. It was not yet initialized though because,
because, because... Hmm, userspace code started and built-in kernel
subsystem was not yet initialized.

> major 5 minor 1. What user space does with that is and has always been a
> problem for user space.
>
> User space is quite at liberty to go .. 5,1 and I have a serial console
> I want to load 8250_pci please.
>
> What it must not do is try and re-open it again and again and again.

Heh, init is not allowed to write something to the console or any other
tty?

--
Evgeniy Polyakov

2008-12-07 17:14:28

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 05:01:08PM +0000, Alan Cox ([email protected]) wrote:
> > Wrong. First at least because tty/console stuff is built in.
>
> Are you two people or just two names and email addresses for
> the same ?

Depending on who you are asking for.

> /dev/console is a logical mapping to a device which may well be
> different, loaded after PCI is initialised and dependant on PCI.

Yup. It may. But in showed case it is not. In showed case it showed a
bug. Which you call a feature.

> > But we alredy got, that you decided that Debian's initramfs-tools (0.92e)
> > are allowed not to boot with some kernel configs.
>
> Yes. They won't boot if you don't include any disk drivers. They won't
> boot if you don't include any file systems. They wont boot in a million
> other cases. That is the user space not the kernel at fault.

But right now _everything_ is presented. All things are in places.
Disk drivers, filesystem, and even that stupid console/tty driver.
It _IS_ in the kernel.

> The kernel is doing precisely what it is supposed to. It is even logging
> the user space bug and stopping trying to get stuck in a loop loading a
> module in order to attempt to cope with the user space bug.
>
> If you want to complain then file a debian bug, go fix the user space.

You introduce so naive rules for the initramfs userspace... It should
not use console, it should not load modules, which may load another
one... What next, not to use syscalls? Sigh, instead of thinking on how
to fix the weak boot process so that next time similar problem arises,
we ended up with pointing a finger one to another... Hope we will not
get another similar cases though.

--
Evgeniy Polyakov

2008-12-07 17:18:01

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 18:01, Alan Cox <[email protected]> wrote:
>> Wrong. First at least because tty/console stuff is built in.
>
> Are you two people or just two names and email addresses for
> the same ?

Damn, now you found that out.

I suggest you get a few more mail addresses too, to back your
non-sensical claims, and try to establish silly userspace rules to
work around an obvious kernel bug. :)

Good luck, looking forward to your creativity,
Kay

2008-12-07 17:23:08

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 18:01, Alan Cox <[email protected]> wrote:
>> Wrong. First at least because tty/console stuff is built in.
>
> Are you two people or just two names and email addresses for
> the same ?
>
> /dev/console is a logical mapping to a device which may well be
> different, loaded after PCI is initialised and dependant on PCI.

So wrong. If no driver is associated, like early, in that case, we
must return -ENODEV, instead of calling modprobe in a loop. It's a
built-in device, and it's easy to fix.

Again, please start to think about, it's all contained in the kernel,
and the kernel is wrong. Claiming userspace is guilty to triggering
this kernel bug does not help anything.

Kay

2008-12-07 17:25:07

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> Heh, init is not allowed to write something to the console or any other
> tty?

init is not run as the helper of a modprobe.

2008-12-07 17:29:02

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> > /dev/console is a logical mapping to a device which may well be
> > different, loaded after PCI is initialised and dependant on PCI.
>
> So wrong. If no driver is associated, like early, in that case, we
> must return -ENODEV, instead of calling modprobe in a loop. It's a
> built-in device, and it's easy to fix.

You've clearly no idea how initrd even works have you ? If it just
returned -ENODEV you wouldn't be able to open the console and you
wouldn't trigger the loading of the module to get the console running. So
you've now completely buggered the boot process.

The correct sequence is

Open device
Kernel issues hotplug message
Hotplug script loads drivers to policy


The problem case you have due to initrd bugs is

Open device
Kernel issues hotplug message
Hotplug script opens same device (BUG)
Kernel issues hotplug message
.....
Kernel detects this is stuck
Kernel replies with -ENODEV/-ENXIO to try
and rescue itself from buggy initrd scripts

That is how it has worked since we first had script based module
requesting which is some years now.

2008-12-07 17:29:25

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 05:24:47PM +0000, Alan Cox ([email protected]) wrote:
> > Heh, init is not allowed to write something to the console or any other
> > tty?
>
> init is not run as the helper of a modprobe.

I hope it is not another limitation for the early userspace? :)
I worked with systems where init made everything. Not showing that as an
example of particulary good design, but still there may be situations
when userspace code does something, which kernel is not ready for. And
it is a kernel problem that it is not ready for some usespace behaviour.

Btw, modprobe can not print error message to the console, that's the
rule ;)

--
Evgeniy Polyakov

2008-12-07 17:40:00

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 18:28, Alan Cox <[email protected]> wrote:
>> > /dev/console is a logical mapping to a device which may well be
>> > different, loaded after PCI is initialised and dependant on PCI.
>>
>> So wrong. If no driver is associated, like early, in that case, we
>> must return -ENODEV, instead of calling modprobe in a loop. It's a
>> built-in device, and it's easy to fix.
>
> You've clearly no idea how initrd even works have you ?

Not sure, if you understand the real problem. A kernel forked binary
is allowed to access /dev/console, but it triggers a kernel bug.

> If it just
> returned -ENODEV you wouldn't be able to open the console and you
> wouldn't trigger the loading of the module to get the console running. So
> you've now completely buggered the boot process.

Utter nonsense. Exactly when the driver is available shortly later,
the console will work. If it's not backed by a driver, it should not
try to load it, it will never get any driver loaded by opening it. The
kernel must handle that.

> The correct sequence is
>
> Open device
> Kernel issues hotplug message
> Hotplug script loads drivers to policy

Nonsense. The kernel calls /sbin/modprobe directly, no hotplug involved.

> The problem case you have due to initrd bugs is
>
> Open device
> Kernel issues hotplug message
> Hotplug script opens same device (BUG)

The kernel calls modprobe for something, modprobe tries to log an
error, and the kernel calls modprobe again. Bug! No hotplug involved.

> Kernel issues hotplug message
> .....
> Kernel detects this is stuck
> Kernel replies with -ENODEV/-ENXIO to try
> and rescue itself from buggy initrd scripts

Totally wrong, It never was that way.

> That is how it has worked since we first had script based module
> requesting which is some years now.

Please update your idea of hotplug and the kernel module loader, you
are on the totally wrong track.

Thanks,
Kay

2008-12-07 17:44:50

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 05:28:55PM +0000, Alan Cox ([email protected]) wrote:
> > > /dev/console is a logical mapping to a device which may well be
> > > different, loaded after PCI is initialised and dependant on PCI.
> >
> > So wrong. If no driver is associated, like early, in that case, we
> > must return -ENODEV, instead of calling modprobe in a loop. It's a
> > built-in device, and it's easy to fix.
>
> You've clearly no idea how initrd even works have you ? If it just
> returned -ENODEV you wouldn't be able to open the console and you
> wouldn't trigger the loading of the module to get the console running. So
> you've now completely buggered the boot process.
>
> The correct sequence is
>
> Open device
> Kernel issues hotplug message
> Hotplug script loads drivers to policy
>
>
> The problem case you have due to initrd bugs is
>
> Open device
> Kernel issues hotplug message
> Hotplug script opens same device (BUG)

Everyone understands that, what you do not want to get, is that this
case can be handled by the kernel so that there would be no recursion.
And instead of thinking how to fix it, you just try to shut it up.

There may be another case, when the same happens inside the kernel, i.e.
module being loaded requires console and the same happens again. Similar
problem exists for network console, when there is no underlying device
yet, but it is handled.

Fortunately console is the most common example (maybe even the only
one), so this case can be fixed easily. Moreover, because of subtle
tty ordering, when everything is in kernel, it still may be triggered,
as was shown previously.

And while having dumb console device sounds awful for you, it is a
bulletprof solution even for non-expected userspace behaviout. And so
far the only objection was that it may break something, which you
believe may happen not even being tested.

Alan, let's make some progress on this fingerpointing. If Herbert's
patch fixes the crypto loading problem, it will find its way upstream
for the current tree, and in the merge window Kay's patch may be applied
and widely tested. Thoughts?

--
Evgeniy Polyakov

2008-12-07 17:51:22

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, 7 Dec 2008 18:39:48 +0100
"Kay Sievers" <[email protected]> wrote:

> On Sun, Dec 7, 2008 at 18:28, Alan Cox <[email protected]> wrote:
> >> > /dev/console is a logical mapping to a device which may well be
> >> > different, loaded after PCI is initialised and dependant on PCI.
> >>
> >> So wrong. If no driver is associated, like early, in that case, we
> >> must return -ENODEV, instead of calling modprobe in a loop. It's a
> >> built-in device, and it's easy to fix.
> >
> > You've clearly no idea how initrd even works have you ?
>
> Not sure, if you understand the real problem. A kernel forked binary
> is allowed to access /dev/console, but it triggers a kernel bug.

If there is a hotplug load for the console device then it cannot. Just as
a request for the driver for /dev/hda cannot open /dev/hda* again.

> Nonsense. The kernel calls /sbin/modprobe directly, no hotplug involved.

Its up to you if the kernel calls modprobe and what your modprobe is

> The kernel calls modprobe for something, modprobe tries to log an
> error, and the kernel calls modprobe again. Bug! No hotplug involved.

A modprobe to load the console device shouln't open /dev/console. Very
simple and always been true.

> > Kernel issues hotplug message
> > .....
> > Kernel detects this is stuck
> > Kernel replies with -ENODEV/-ENXIO to try
> > and rescue itself from buggy initrd scripts
>
> Totally wrong, It never was that way

Funny but its been that way for many many years. Just as it cannot try and
syslog an error when loading the AF_UNIX socket family.

Alan

2008-12-07 17:52:45

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> Alan, let's make some progress on this fingerpointing. If Herbert's
> patch fixes the crypto loading problem, it will find its way upstream
> for the current tree, and in the merge window Kay's patch may be applied
> and widely tested. Thoughts?

I have no intention of applying Kay's patch because it is wrong and it
will only break things not fix them.

Alan

2008-12-07 17:54:49

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 05:52:45PM +0000, Alan Cox ([email protected]) wrote:
> > Alan, let's make some progress on this fingerpointing. If Herbert's
> > patch fixes the crypto loading problem, it will find its way upstream
> > for the current tree, and in the merge window Kay's patch may be applied
> > and widely tested. Thoughts?
>
> I have no intention of applying Kay's patch because it is wrong and it
> will only break things not fix them.

And you are sure it breaks something based on what?

--
Evgeniy Polyakov

2008-12-07 18:03:14

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, 7 Dec 2008 20:54:38 +0300
Evgeniy Polyakov <[email protected]> wrote:

> On Sun, Dec 07, 2008 at 05:52:45PM +0000, Alan Cox ([email protected]) wrote:
> > > Alan, let's make some progress on this fingerpointing. If Herbert's
> > > patch fixes the crypto loading problem, it will find its way upstream
> > > for the current tree, and in the merge window Kay's patch may be applied
> > > and widely tested. Thoughts?
> >
> > I have no intention of applying Kay's patch because it is wrong and it
> > will only break things not fix them.
>
> And you are sure it breaks something based on what?


Firstly:

You propose to implement

modprobe fails (due to crypto requirements)
open /dev/console
-ENODEV
log error to nowhere


Why is this useful - you now get failing module loads producing no
diagnostics and in many case the setup just dying silently. Previously
you got an attempt to recover and diagnostics which allowed the problem
to be found (as Herbert did)

Secondly:

If I have a /dev/console on a PCI device and I have modprobe set to load
8250_pci or a framebuffer driver when the console is opened you will
break the current working behaviour.

2008-12-07 18:13:26

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 19:03, Alan Cox <[email protected]> wrote:
> On Sun, 7 Dec 2008 20:54:38 +0300
> Evgeniy Polyakov <[email protected]> wrote:
>
>> On Sun, Dec 07, 2008 at 05:52:45PM +0000, Alan Cox ([email protected]) wrote:
>> > > Alan, let's make some progress on this fingerpointing. If Herbert's
>> > > patch fixes the crypto loading problem, it will find its way upstream
>> > > for the current tree, and in the merge window Kay's patch may be applied
>> > > and widely tested. Thoughts?
>> >
>> > I have no intention of applying Kay's patch because it is wrong and it
>> > will only break things not fix them.
>>
>> And you are sure it breaks something based on what?
>
>
> Firstly:
>
> You propose to implement
>
> modprobe fails (due to crypto requirements)
> open /dev/console
> -ENODEV
> log error to nowhere

Yes, log nowhere instead of running in a loop would be much better
than loading a 5:1 driver which will never exist as a module.

> Why is this useful - you now get failing module loads producing no
> diagnostics and in many case the setup just dying silently.

It's obviously more useful than not to boot up.

> Previously
> you got an attempt to recover and diagnostics which allowed the problem
> to be found (as Herbert did)
>
> Secondly:
>
> If I have a /dev/console on a PCI device and I have modprobe set to load
> 8250_pci or a framebuffer driver when the console is opened you will
> break the current working behaviour.

No, the pci driver will never get loaded by modprobe 5:1, that's all
what this bug is about. It connects to the existing console device
when the driver is loaded, and at that moment /dev/console gets
working.

Kay

2008-12-07 18:16:00

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> Yes, log nowhere instead of running in a loop would be much better
> than loading a 5:1 driver which will never exist as a module.

The loop is detected and terminated.
>
> > Why is this useful - you now get failing module loads producing no
> > diagnostics and in many case the setup just dying silently.
>
> It's obviously more useful than not to boot up.

What makes you think it will now boot up. The loop is already detected
and terminated. What will you do if it doesn't and you get no
diagnostics. How will distributions debug those reports in bugzilla.

> No, the pci driver will never get loaded by modprobe 5:1,

Why not ? You have no idea how the other millions of Linux users have
their module loading rules configured. A change which breaks this
behaviour is a regression.

Alan

2008-12-07 18:22:08

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 19:15, Alan Cox <[email protected]> wrote:
>> Yes, log nowhere instead of running in a loop would be much better
>> than loading a 5:1 driver which will never exist as a module.
>
> The loop is detected and terminated.

No. Please back up what you are trying to talk about.

>> > Why is this useful - you now get failing module loads producing no
>> > diagnostics and in many case the setup just dying silently.
>>
>> It's obviously more useful than not to boot up.
>
> What makes you think it will now boot up. The loop is already detected
> and terminated. What will you do if it doesn't and you get no
> diagnostics. How will distributions debug those reports in bugzilla.

The boxes of the reporters hang! Read the bug! Please!

>> No, the pci driver will never get loaded by modprobe 5:1,
>
> Why not ? You have no idea how the other millions of Linux users have
> their module loading rules configured. A change which breaks this
> behaviour is a regression.

There is no cheap way out of the problem, it's a kernel bug, and we
will fix it - you may just delay it with your zero arguments.

Kay

2008-12-07 18:22:54

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 18:51, Alan Cox <[email protected]> wrote:
> On Sun, 7 Dec 2008 18:39:48 +0100
> "Kay Sievers" <[email protected]> wrote:
>
>> On Sun, Dec 7, 2008 at 18:28, Alan Cox <[email protected]> wrote:
>> >> > /dev/console is a logical mapping to a device which may well be
>> >> > different, loaded after PCI is initialised and dependant on PCI.
>> >>
>> >> So wrong. If no driver is associated, like early, in that case, we
>> >> must return -ENODEV, instead of calling modprobe in a loop. It's a
>> >> built-in device, and it's easy to fix.
>> >
>> > You've clearly no idea how initrd even works have you ?
>>
>> Not sure, if you understand the real problem. A kernel forked binary
>> is allowed to access /dev/console, but it triggers a kernel bug.
>
> If there is a hotplug load for the console device then it cannot. Just as
> a request for the driver for /dev/hda cannot open /dev/hda* again.

So wrong. Modprobe 5:1 will never load anything, but kill the box in
such case at that time of bootup.

>> Nonsense. The kernel calls /sbin/modprobe directly, no hotplug involved.
>
> Its up to you if the kernel calls modprobe and what your modprobe is

What are you saying? Your picture of hotplug is just wrong, regardless
of "it's up to you". I talk about what people use, and what is
obviously broken currently.

>> The kernel calls modprobe for something, modprobe tries to log an
>> error, and the kernel calls modprobe again. Bug! No hotplug involved.
>
> A modprobe to load the console device shouln't open /dev/console. Very
> simple and always been true.

We are _not_ loading a console driver that way, we try to load the
console device itself, not the driver. There is no driver for 5:1 in
any module.

>> > Kernel issues hotplug message
>> > .....
>> > Kernel detects this is stuck
>> > Kernel replies with -ENODEV/-ENXIO to try
>> > and rescue itself from buggy initrd scripts
>>
>> Totally wrong, It never was that way
>
> Funny but its been that way for many many years. Just as it cannot try and
> syslog an error when loading the AF_UNIX socket family.

And? Keep on the subject please. It's still a bug to run in a loop
trying something that will _never_ exist.

Kay

2008-12-07 18:34:37

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> > The loop is detected and terminated.
>
> No. Please back up what you are trying to talk about.

Let me introduce you to.. drum roll.. the source code. Its a useful
resource, why don't you use it for once.


max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
atomic_inc(&kmod_concurrent);
if (atomic_read(&kmod_concurrent) > max_modprobes) {
/* We may be blaming an innocent here, but unlikely */
if (kmod_loop_msg++ < 5)
printk(KERN_ERR
"request_module: runaway loop modprobe
%s\n", module_name);
atomic_dec(&kmod_concurrent);
return -ENOMEM;
}

Happy now. Print it out, share it with friends, find someone who can read
C if you are stuck.

> The boxes of the reporters hang! Read the bug! Please!

They would still hang. As I repeatedly said for the benefit of two people
who don't seem to be able to read source code, the loop is detected and
terminated. So it already fails the open when it sees it has gotten five
layers deep.

> There is no cheap way out of the problem, it's a kernel bug, and we
> will fix it - you may just delay it with your zero arguments.

Oh I see. Allow me to explain your position in the words of some small
children I know

"ME!! ME!! ME!! ME!! ME!!"

I don't care about your obscure corner-case non bug that in fact was a
crypto bug combined with a modprobe bug and where the crypto bug is
now fixed. I do care about not breaking existing users systems. The fact
we do this is why Linux doesn't suck.

Alan

2008-12-07 19:02:26

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 19:31, Alan Cox <[email protected]> wrote:
>> > The loop is detected and terminated.
>>
>> No. Please back up what you are trying to talk about.
>
> Let me introduce you to.. drum roll.. the source code. Its a useful
> resource, why don't you use it for once.
>
>
> max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
> atomic_inc(&kmod_concurrent);
> if (atomic_read(&kmod_concurrent) > max_modprobes) {
> /* We may be blaming an innocent here, but unlikely */
> if (kmod_loop_msg++ < 5)
> printk(KERN_ERR
> "request_module: runaway loop modprobe
> %s\n", module_name);
> atomic_dec(&kmod_concurrent);
> return -ENOMEM;
> }
>
> Happy now. Print it out, share it with friends, find someone who can read
> C if you are stuck.

It does not work, that's all. Reproduce the bug and look at it for yourself.

It's still a bug, regardless of all the childish stuff you wrap around it.

>> The boxes of the reporters hang! Read the bug! Please!
>
> They would still hang. As I repeatedly said for the benefit of two people
> who don't seem to be able to read source code, the loop is detected and
> terminated. So it already fails the open when it sees it has gotten five
> layers deep.
>
>> There is no cheap way out of the problem, it's a kernel bug, and we
>> will fix it - you may just delay it with your zero arguments.
>
> Oh I see. Allow me to explain your position in the words of some small
> children I know
>
> "ME!! ME!! ME!! ME!! ME!!"
>
> I don't care about your obscure corner-case non bug that in fact was a
> crypto bug combined with a modprobe bug and where the crypto bug is
> now fixed. I do care about not breaking existing users systems. The fact
> we do this is why Linux doesn't suck.

It's not obscure, it's obviously broken. And it currently sucks.

I have no idea why are you repeatedly, with completely wrong arguments
trying, to explain "hotplug" stuff to me. I maintain it by the way,
and I didn't see you involved here in the last years, so I guess you
miss the background to understand the problem.

Kay

2008-12-07 20:00:24

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> > max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
> > atomic_inc(&kmod_concurrent);
> > if (atomic_read(&kmod_concurrent) > max_modprobes) {
> > /* We may be blaming an innocent here, but unlikely */
> > if (kmod_loop_msg++ < 5)
> > printk(KERN_ERR
> > "request_module: runaway loop modprobe
> > %s\n", module_name);
> > atomic_dec(&kmod_concurrent);
> > return -ENOMEM;
> > }
> >
> > Happy now. Print it out, share it with friends, find someone who can read
> > C if you are stuck.
>
> It does not work, that's all.

It works for me.

> Reproduce the bug and look at it for yourself.

Well since you've got a reproducer and this code works for me (I've tested
it just fine), why don't you go and reproduce the problem then post a fix
to that code I quoted instead of all this reordering rubbish. If you fix
this code not only won't you risk all the mess from re-ordering
initialisations around the kernel but you'll fix non console related
looping which you imply is also broken as you claim that code doesn't
work for you.

If I deliberately break my module utils I see a sequence of modprobes
which then hits kmod_concurrent limit then causes a -ENOMEM back to
userspace which then fails the file open. The bug report also shows the
printk is displayed so the runaway loop *was* detected and the code paths
taken which stopped the loop.

I get open -> modprobe -> open -> modprobe -> open -> modprobe ... ->
open fail, then open fail, open fail, open fail, open fail back to the
first modprobe exiting.

Your proposal to keep the current recent modprobe parameter strings would
shorten the amount of recursion but it wouldn't change the result that I
can see. If I open /dev/console early and wrongly from a modprobe then I
ultimately get a failing open just as I should do.

Alan

2008-12-07 22:26:55

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 7, 2008 at 21:00, Alan Cox <[email protected]> wrote:
>> > max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
>> > atomic_inc(&kmod_concurrent);
>> > if (atomic_read(&kmod_concurrent) > max_modprobes) {
>> > /* We may be blaming an innocent here, but unlikely */
>> > if (kmod_loop_msg++ < 5)
>> > printk(KERN_ERR
>> > "request_module: runaway loop modprobe
>> > %s\n", module_name);
>> > atomic_dec(&kmod_concurrent);
>> > return -ENOMEM;
>> > }
>> >
>> > Happy now. Print it out, share it with friends, find someone who can read
>> > C if you are stuck.
>>
>> It does not work, that's all.
>
> It works for me.

It runs in a loop here, just like the $subject says, and bko#12153 says.

>> Reproduce the bug and look at it for yourself.
>
> Well since you've got a reproducer and this code works for me (I've tested
> it just fine), why don't you go and reproduce the problem then post a fix
> to that code I quoted instead of all this reordering rubbish. If you fix
> this code not only won't you risk all the mess from re-ordering
> initialisations around the kernel but you'll fix non console related
> looping which you imply is also broken as you claim that code doesn't
> work for you.
>
> If I deliberately break my module utils I see a sequence of modprobes
> which then hits kmod_concurrent limit then causes a -ENOMEM back to
> userspace which then fails the file open. The bug report also shows the
> printk is displayed so the runaway loop *was* detected and the code paths
> taken which stopped the loop.

Sure, I can try to find out why the limiter does does not work here.

> I get open -> modprobe -> open -> modprobe -> open -> modprobe ... ->
> open fail, then open fail, open fail, open fail, open fail back to the
> first modprobe exiting.
>
> Your proposal to keep the current recent modprobe parameter strings would
> shorten the amount of recursion but it wouldn't change the result that I
> can see. If I open /dev/console early and wrongly from a modprobe then I
> ultimately get a failing open just as I should do.

No, my proposal would prevent the kernel to call any modprobe for the
special built-in device 5:1, it shortens the recursion to zero, which
sounds like the right fix. It's really weird to fix the symptoms,
instead of the real problem.

The kernel forks a binary with a broken environment. Even the
in-kernel-tree cpio generator creates /dev/console, and accessing it
from a kernel forked binary makes it crash. The kernel must provide a
sane environment for stuff that it calls, I think, that is pretty
obvious.

Device 5:1 is a core device, which never makes sense to call modprobe
for it. No other later driver will ever register that dev_t, so we
should do it before calling out to other random userspace stuff which
triggers the kernel to go crazy with its own devices.

My proposal was to connect 5:1 to the kobject map at the same time as
we register the whole tty class. We allocate the class, create sysfs
entries, run /sbin/modprobe at that time, why shouldn't we just
register the dev_t that time too?

It properly prevents the needless userspace "driver searching" for a
well known, already created, but not properly registered core device.

It makes the process environment sane, and prevents the root cause of
the bug, and does not just limit the damage.

Thanks,
Kay

2008-12-08 01:19:16

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

Let's take a step back, shall we? Fundamentally, what's going on here
is that a particular distribution's initrd (Debian's, to be precise)
is running into an error in response to a modprobe request for
char-major-5-1, and it is attempting to write to the console, which is
resulting in another modprobe request.... ad infinitum.

There is a dispute about whether it is looping forever, or whether it
should be getting caught by kernel/kmod.c's modprobe recursion
detector. Alan has checked the recursion detector and reports that it
works just fine; Evgeniy and Kay are claiming that it in fact loops
forever, and the recursion detector is not working. I'm going to
guess that Alan tested on Fedora, where it did work just fine, and
reason why people using a Debian-derived initrd is seeing a recursive
loop is because the recursive loop detector works by detecting up to
five concurrent calls to modprobe. That is, while the userspace
helper process is running, another userspace helper is invoked, and so
on, so that there are five userspace helpers piled up on one another,
this will trigger the automatic recursion detector. I'm guessing why
it isn't working given Debian's initrd setup is that whatever is
ultimately opening /dev/console isn't being called until after the
helper script has exited.

Given that the general purpose recursion detector is apparently not
working at least in this case, Kay has proposed that we special case a
kludge wihch prevents the userspace helper be called in the case of
5:1. His argument in favor of doing this is that /dev/console is
never a module, so requesting char-major-5-1 will never be helpful,
and this error can only happen in early userspace, when the tty
subsystem hasn't been initialized yet. Alan claims this could also
happen if the appropriate low-level console driver hasn't been loaded,
and so perhaps the right thing in response to the request for
char-major-5-1 is to load 8250_pci. Here, I think Alan is wrong, and
Kay is right. From looking at the source, if there is no low-level
console driver loaded, there is no call to request_module(); the only
time this can happen is when tty driver hasn't been initialized in
early startup.

On the other hand, Alan is right that in general it is the usermode
helper and initrd's responsibility not create a recursive dependency.
This is in general true, not just for /dev/console. So based on that,
it can be argued that the recursion kludge checking for 5:1 should
just as much be put in userspace. In addition, the fact that
recursion detection isn't working also seems to indicate that initrd
in question is also doing something very wrong.

So I would think the best thing to do is to figure out what Debian's
initrd is doing that is evading the recursion detection. Fixing that
is going to make things much more robust.

Regards,

- Ted

2008-12-08 03:24:40

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, 07 Dec 2008 19:22:41 +0100, Kay Sievers said:

> We are _not_ loading a console driver that way, we try to load the
> console device itself, not the driver. There is no driver for 5:1 in
> any module.

Semantic quibbling, trying to paper over stupidity.

It doesn't really matter if it's a "console driver" or "the console device
itself". If you're in modprobe loading *any* piece of "all the stuff needed
to make open("/dev/console") work", the last thing you want to be doing
is opening /dev/console to complain about something not working.

As Alan said several times now, there really *is* a requirement that early
userspace has its act together.


Attachments:
(No filename) (226.00 B)

2008-12-08 03:35:34

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Mon, Dec 8, 2008 at 02:18, Theodore Tso <[email protected]> wrote:
> Let's take a step back, shall we? Fundamentally, what's going on here
> is that a particular distribution's initrd (Debian's, to be precise)
> is running into an error in response to a modprobe request for
> char-major-5-1, and it is attempting to write to the console, which is
> resulting in another modprobe request.... ad infinitum.
>
> There is a dispute about whether it is looping forever, or whether it
> should be getting caught by kernel/kmod.c's modprobe recursion
> detector. Alan has checked the recursion detector and reports that it
> works just fine; Evgeniy and Kay are claiming that it in fact loops
> forever, and the recursion detector is not working. I'm going to
> guess that Alan tested on Fedora, where it did work just fine, and
> reason why people using a Debian-derived initrd is seeing a recursive
> loop is because the recursive loop detector works by detecting up to
> five concurrent calls to modprobe. That is, while the userspace
> helper process is running, another userspace helper is invoked, and so
> on, so that there are five userspace helpers piled up on one another,
> this will trigger the automatic recursion detector. I'm guessing why
> it isn't working given Debian's initrd setup is that whatever is
> ultimately opening /dev/console isn't being called until after the
> helper script has exited.
>
> Given that the general purpose recursion detector is apparently not
> working at least in this case, Kay has proposed that we special case a
> kludge wihch prevents the userspace helper be called in the case of
> 5:1. His argument in favor of doing this is that /dev/console is
> never a module, so requesting char-major-5-1 will never be helpful,
> and this error can only happen in early userspace, when the tty
> subsystem hasn't been initialized yet. Alan claims this could also
> happen if the appropriate low-level console driver hasn't been loaded,
> and so perhaps the right thing in response to the request for
> char-major-5-1 is to load 8250_pci. Here, I think Alan is wrong, and
> Kay is right. From looking at the source, if there is no low-level
> console driver loaded, there is no call to request_module(); the only
> time this can happen is when tty driver hasn't been initialized in
> early startup.
>
> On the other hand, Alan is right that in general it is the usermode
> helper and initrd's responsibility not create a recursive dependency.
> This is in general true, not just for /dev/console. So based on that,
> it can be argued that the recursion kludge checking for 5:1 should
> just as much be put in userspace. In addition, the fact that
> recursion detection isn't working also seems to indicate that initrd
> in question is also doing something very wrong.
>
> So I would think the best thing to do is to figure out what Debian's
> initrd is doing that is evading the recursion detection. Fixing that
> is going to make things much more robust.

I tested it with an initramfs image with /sbin/modprobe being a shell
script that writes a few bytes to /dev/console, and does nothing else.
It did run for minutes here, until I stopped it.

Sure, if we can make userspace behave nicely, I'm all for doing it. On
the other hand, I think it's a good thing to provide a sane
environment by the kernel for any "non-initramfs-optimized" helper.
Writing to /dev/console is a usual behavior for tools used in
initramfs, to be able to debug bootup problems. In many cases it is
just glibc's fallback LOG_CONS, if syslog is not available.

Modprobe has syslog code, so it might very well be, that the Debian
initramfs isn't doing anything obscure. The current behavior, is very
easy to trigger bug, and a pretty hard to debug setup, if you do not
add debugging code to the kernel code itself.

That's why I still think it's a good thing, to connect the core tty
devices to their dev_t handler internally, before we init all the
other drivers and run userspace. It will prevent any further needless
searches by the kernel, for a driver for the already created but just
not registered tty devices. We will do exactly the same dev_t
<->device connection (register_chrdev_region) anyway, directly after
loading all the other drivers.

Thanks,
Kay

2008-12-08 03:56:23

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Mon, Dec 8, 2008 at 04:23, <[email protected]> wrote:
> On Sun, 07 Dec 2008 19:22:41 +0100, Kay Sievers said:
>
>> We are _not_ loading a console driver that way, we try to load the
>> console device itself, not the driver. There is no driver for 5:1 in
>> any module.

[childish blurb removed]

> It doesn't really matter if it's a "console driver" or "the console device
> itself". If you're in modprobe loading *any* piece of "all the stuff needed
> to make open("/dev/console") work", the last thing you want to be doing
> is opening /dev/console to complain about something not working.

Nothing in initramfs or userspace tries to load "all the stuff needed
to make open("/dev/console") work". It's the kernel itself, that tries
to resolve its own requirements.

The kernel forked binary _writes_ to /dev/console, if we like it or
not, it seems to do that, and it's arguable why it should not, or how
it should detect that it is not allowed to do that. You may just
depend on the logging of binaries to find other bugs.

The helper was called in the first place for something else, in this
case the "cryptomgr". The loop is caused entirely by the kernel
itself, if /dev/console is just accessed. There is no intentional
loading of any console driver from userspace happening here.

Kay

2008-12-08 13:07:00

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Sun, Dec 07, 2008 at 11:55:07PM +0800, Herbert Xu ([email protected]) wrote:
> Having said that I think this patch should remove this particular
> trigger. Unfortunately I couldn't find a more succinct way of
> expressing this relationship: if ALGAPI is built-in because it's
> selected by an algorithm, and CRYPTO_MANAGER is enabled then we
> require CRYPTO_MANAGER to be built-in as well.
>
> Evgeniy, please let me know whether this fixes the problem.

Your patch does not apply cleanly to the latest git and missed
rng dependency to rng2.
Attached patch fixes problem with module loading for me.

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 39dbd8e..dc20a34 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -31,35 +31,63 @@ config CRYPTO_FIPS

config CRYPTO_ALGAPI
tristate
+ select CRYPTO_ALGAPI2
help
This option provides the API for cryptographic algorithms.

+config CRYPTO_ALGAPI2
+ tristate
+
config CRYPTO_AEAD
tristate
+ select CRYPTO_AEAD2
select CRYPTO_ALGAPI

+config CRYPTO_AEAD2
+ tristate
+ select CRYPTO_ALGAPI2
+
config CRYPTO_BLKCIPHER
tristate
+ select CRYPTO_BLKCIPHER2
select CRYPTO_ALGAPI
- select CRYPTO_RNG
+
+config CRYPTO_BLKCIPHER2
+ tristate
+ select CRYPTO_ALGAPI2
+ select CRYPTO_RNG2

config CRYPTO_HASH
tristate
+ select CRYPTO_HASH2
select CRYPTO_ALGAPI

+config CRYPTO_HASH2
+ tristate
+ select CRYPTO_ALGAPI2
+
config CRYPTO_RNG
tristate
+ select CRYPTO_RNG2
select CRYPTO_ALGAPI

+config CRYPTO_RNG2
+ tristate
+ select CRYPTO_ALGAPI2
+
config CRYPTO_MANAGER
tristate "Cryptographic algorithm manager"
- select CRYPTO_AEAD
- select CRYPTO_HASH
- select CRYPTO_BLKCIPHER
+ select CRYPTO_MANAGER2
help
Create default cryptographic template instantiations such as
cbc(aes).

+config CRYPTO_MANAGER2
+ def_tristate CRYPTO_MANAGER || (CRYPTO_MANAGER!=n && CRYPTO_ALGAPI=y)
+ select CRYPTO_AEAD2
+ select CRYPTO_HASH2
+ select CRYPTO_BLKCIPHER2
+
config CRYPTO_GF128MUL
tristate "GF(2^128) multiplication functions (EXPERIMENTAL)"
depends on EXPERIMENTAL
diff --git a/crypto/Makefile b/crypto/Makefile
index 5862b80..cd4a4ed 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -9,24 +9,24 @@ obj-$(CONFIG_CRYPTO_FIPS) += fips.o

crypto_algapi-$(CONFIG_PROC_FS) += proc.o
crypto_algapi-objs := algapi.o scatterwalk.o $(crypto_algapi-y)
-obj-$(CONFIG_CRYPTO_ALGAPI) += crypto_algapi.o
+obj-$(CONFIG_CRYPTO_ALGAPI2) += crypto_algapi.o

-obj-$(CONFIG_CRYPTO_AEAD) += aead.o
+obj-$(CONFIG_CRYPTO_AEAD2) += aead.o

crypto_blkcipher-objs := ablkcipher.o
crypto_blkcipher-objs += blkcipher.o
-obj-$(CONFIG_CRYPTO_BLKCIPHER) += crypto_blkcipher.o
-obj-$(CONFIG_CRYPTO_BLKCIPHER) += chainiv.o
-obj-$(CONFIG_CRYPTO_BLKCIPHER) += eseqiv.o
+obj-$(CONFIG_CRYPTO_BLKCIPHER2) += crypto_blkcipher.o
+obj-$(CONFIG_CRYPTO_BLKCIPHER2) += chainiv.o
+obj-$(CONFIG_CRYPTO_BLKCIPHER2) += eseqiv.o
obj-$(CONFIG_CRYPTO_SEQIV) += seqiv.o

crypto_hash-objs := hash.o
crypto_hash-objs += ahash.o
-obj-$(CONFIG_CRYPTO_HASH) += crypto_hash.o
+obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o

cryptomgr-objs := algboss.o testmgr.o

-obj-$(CONFIG_CRYPTO_MANAGER) += cryptomgr.o
+obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
obj-$(CONFIG_CRYPTO_HMAC) += hmac.o
obj-$(CONFIG_CRYPTO_XCBC) += xcbc.o
obj-$(CONFIG_CRYPTO_NULL) += crypto_null.o
@@ -73,8 +73,8 @@ obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
obj-$(CONFIG_CRYPTO_CRC32C) += crc32c.o
obj-$(CONFIG_CRYPTO_AUTHENC) += authenc.o
obj-$(CONFIG_CRYPTO_LZO) += lzo.o
-obj-$(CONFIG_CRYPTO_RNG) += rng.o
-obj-$(CONFIG_CRYPTO_RNG) += krng.o
+obj-$(CONFIG_CRYPTO_RNG2) += rng.o
+obj-$(CONFIG_CRYPTO_RNG2) += krng.o
obj-$(CONFIG_CRYPTO_ANSI_CPRNG) += ansi_cprng.o
obj-$(CONFIG_CRYPTO_TEST) += tcrypt.o

--
Evgeniy Polyakov

2008-12-08 13:22:21

by Evgeniy Polyakov

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

Hi.

On Sat, Dec 06, 2008 at 08:32:06PM +0100, Kay Sievers ([email protected]) wrote:
> I don't know if it may have any bad side-effects. It moves the tty
> registration earlier, before we do pci, framebuffer, video, acpi
> registration.
>
> It boots fine here with and without initramfs.
>
> Maybe it makes the "dead" /dev/console in your initramfs working, then
> we at least know the problem.

Your patch fixes boot process in my case.

--
Evgeniy Polyakov

2008-12-09 00:42:49

by Herbert Xu

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Mon, Dec 08, 2008 at 04:06:46PM +0300, Evgeniy Polyakov wrote:
>
> Your patch does not apply cleanly to the latest git and missed
> rng dependency to rng2.

Right it was against cryptodev-2.6 actually. Thanks for fixing
it up and confirming. I'll submit this to Linus.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-12-09 01:09:59

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Mon, Dec 08, 2008 at 04:35:20AM +0100, Kay Sievers wrote:
> Sure, if we can make userspace behave nicely, I'm all for doing it. On
> the other hand, I think it's a good thing to provide a sane
> environment by the kernel for any "non-initramfs-optimized" helper.
> Writing to /dev/console is a usual behavior for tools used in
> initramfs, to be able to debug bootup problems. In many cases it is
> just glibc's fallback LOG_CONS, if syslog is not available.

Well, can we agree that (a) there is a generalized modprobe recursion
detector already in the kernel, and (b) for some reason, Debian isn't
triggering it? That seems like a bug, and while it may not be 100%
clear whether the bug is on the userspace side or the kernel side, it
would seem that finding and fixing *that* would be a Good Thing, and
it could be argued that a specific don't request char-major-5-1 would
be papering over this bug (whether it is in Debian's initrd or in the
kernel side code). It would be good to make sure we understand what
the root causes for while the modprobe recursion detector is
apparently not triggering, since it could be that Debian's initrd
might cause some other uncaught recursion loop if we don't drive this
problem determination to root cause.

> That's why I still think it's a good thing, to connect the core tty
> devices to their dev_t handler internally, before we init all the
> other drivers and run userspace.

Um, how early? (/me searches lkml.org for the original patch). Ah,
OK, you want to do it postcore.... There may actually be a problem
with that, because it looks like vtconsole_class_init (which is
currently run as a postcore initcall) really wants to happen before
the tty layer initializes itself. So moving tty into postcore could
potentially run into problems, depending on whether vt.c's or
tty_io.c's initcalls are run first.

- Ted

2008-12-09 02:00:18

by Kay Sievers

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

On Tue, Dec 9, 2008 at 02:09, Theodore Tso <[email protected]> wrote:
> On Mon, Dec 08, 2008 at 04:35:20AM +0100, Kay Sievers wrote:
>> Sure, if we can make userspace behave nicely, I'm all for doing it. On
>> the other hand, I think it's a good thing to provide a sane
>> environment by the kernel for any "non-initramfs-optimized" helper.
>> Writing to /dev/console is a usual behavior for tools used in
>> initramfs, to be able to debug bootup problems. In many cases it is
>> just glibc's fallback LOG_CONS, if syslog is not available.
>
> Well, can we agree that (a) there is a generalized modprobe recursion
> detector already in the kernel, and (b) for some reason, Debian isn't
> triggering it? That seems like a bug, and while it may not be 100%
> clear whether the bug is on the userspace side or the kernel side, it
> would seem that finding and fixing *that* would be a Good Thing, and
> it could be argued that a specific don't request char-major-5-1 would
> be papering over this bug (whether it is in Debian's initrd or in the
> kernel side code). It would be good to make sure we understand what
> the root causes for while the modprobe recursion detector is
> apparently not triggering, since it could be that Debian's initrd
> might cause some other uncaught recursion loop if we don't drive this
> problem determination to root cause.

Right, it would be good to know for sure what's going on.

In my test script it was just a simple access to /dev/console, so it
might just be that modprobe wants to log something on Debian. At least
that would match the behavior I've seen here.

And we really rely on these tools to be able to print debug messages
sometimes to find other bugs (at least at the time it's theoretically
possible to log). And I don't see how modprobe could find out, that it
is not allowed to access /dev/console.

As an ideal solution, I would like to have /dev/console working before
_any_ userspace is called, and it would put all the stuff that is
written to it the kernel "dmesg buffer" as long as the console isn't
working, just to be able to see what's going on, if needed. As the
second choice, it would return -ENODEV. The current looping is
definitely not what we want. :)

>> That's why I still think it's a good thing, to connect the core tty
>> devices to their dev_t handler internally, before we init all the
>> other drivers and run userspace.
>
> Um, how early? (/me searches lkml.org for the original patch). Ah,
> OK, you want to do it postcore.... There may actually be a problem
> with that, because it looks like vtconsole_class_init (which is
> currently run as a postcore initcall) really wants to happen before
> the tty layer initializes itself. So moving tty into postcore could
> potentially run into problems, depending on whether vt.c's or
> tty_io.c's initcalls are run first.

Sure, we might need to change that, if it is a problem. It was mainly
to find out what was going on, because people thought that some serial
driver is involved in the loop, which I didn't believe.

To be safer, maybe we just want to move the cdev_add(),
register_chrdev_region() call, which should prevent the "userspace
driver search".

Thanks,
Kay

2008-12-09 10:14:18

by Alan

[permalink] [raw]
Subject: Re: Runaway loop with the current git.

> apparently not triggering, since it could be that Debian's initrd
> might cause some other uncaught recursion loop if we don't drive this
> problem determination to root cause.

Agreed - and there are all sorts of obscure and wonderful other ways to
trip this up which need the detector (AF_UNIX modular and doing a syslog
for example).

> > That's why I still think it's a good thing, to connect the core tty
> > devices to their dev_t handler internally, before we init all the
> > other drivers and run userspace.
>
> Um, how early? (/me searches lkml.org for the original patch). Ah,
> OK, you want to do it postcore.... There may actually be a problem
> with that, because it looks like vtconsole_class_init (which is
> currently run as a postcore initcall) really wants to happen before
> the tty layer initializes itself. So moving tty into postcore could
> potentially run into problems, depending on whether vt.c's or
> tty_io.c's initcalls are run first.

Its playing with fire and doing so to paper over a completely different
problem. The recursion detector is generic and should be sufficient. If
it isn't then it needs improving to be generic, sufficient and working.