2014-10-01 04:54:19

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Tue, Sep 30, 2014 at 8:16 PM, Rob Landley <[email protected]> wrote:
> On 09/30/14 20:52, Frank Rowand wrote:
>> On 9/30/2014 5:58 PM, Rob Landley wrote:
>>> If you're going to argue that it should "default y", that's a defensible
>>> choice. But please don't argue for kernel config symbols with a negative
>>> meaning or we'll start having allyesconfig_n brain damage too...
>>
>> Yes, "default y" is a valid answer to my request.
>
> Works for me.
>
>>>> Instead of using a config option, would adding another kernel
>>>> command line option, such as 'init_fail_is_fatal', work for
>>>> your needs?
>>>
>>> That was the previous series of patches you ignored, which added code so
>>> you can provide _extra_ kernel commands to tell it _not_ to do stuff.
>>> The patches did not generate noticeable enthusiasm.
>>
>> But there also was not a strong push back either. Just Chuck's suggestion
>> of an alternate syntax, and your suggestion of instead using a config
>> option (and possibly immediately deprecating the config option).
>>
>> You could as easily frame the argument that the added code was to
>> tell the kernel to "_do_ stuff" (panic) instead of "_not_ do stuff".
>> But that is just semantics on my part; whatever.
>>
>> I thought the general trend was to try to avoid adding config options.
>> The strictinit method seems fine to me.
>
> Embedded guys care:
>
> http://elinux.org/Linux_Tiny
>
> http://lkml.iu.edu//hypermail/linux/kernel/1409.2/03763.html
>
>>>> I have a feeling this has already been proposed,
>>>> as the 'strictinit' option mentioned in the changes from v3
>>>> below might be the same concept?
>>>
>>> That was it, yes.
>>>
>>> Having to get your kernel config right (and your kernel command line
>>> right) in order for your system to boot is not really a new concept, is
>>> it? You can still specify "init=/bin/sh" if you want that. (I do it all
>>> the time when I need to edit a system I haven't bothered to look up the
>>> root password to.)
>>
>> Yes, of course I can. So it falls back to personal preference (as I said,
>> I like that some failed boots will drop into a shell without having to
>> change the kernel command line).
>
> The config option lets it do that. Default Y preserves the old behavior.

I significantly prefer default N. Scripts that play with init= really
don't want the fallback, and I can imagine contexts in which it could
be a security problem.

--Andy


2014-10-01 18:05:19

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
> I significantly prefer default N. Scripts that play with init= really
> don't want the fallback, and I can imagine contexts in which it could
> be a security problem.

While I certainly would prefer the non-fallback behavior for init as
well, standard kernel practice has typically been to use "default y" for
previously built-in features that become configurable. And I'd
certainly prefer a compile-time configuration option like this (even
with default y) over a "strictinit" kernel command-line option.

- Josh Triplett

2014-10-01 18:13:38

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
> On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
>> I significantly prefer default N. Scripts that play with init= really
>> don't want the fallback, and I can imagine contexts in which it could
>> be a security problem.
>
> While I certainly would prefer the non-fallback behavior for init as
> well, standard kernel practice has typically been to use "default y" for
> previously built-in features that become configurable. And I'd
> certainly prefer a compile-time configuration option like this (even
> with default y) over a "strictinit" kernel command-line option.
>

Fair enough.

So: "default y" for a release or two, then switch the default? Having
default y will annoy virtme, though it's not the end of the world.
Virtme is intended to work with more-or-less-normal kernels.

--Andy

2014-10-01 22:42:34

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Wed, Oct 01, 2014 at 11:13:14AM -0700, Andy Lutomirski wrote:
> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
> > On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
> >> I significantly prefer default N. Scripts that play with init= really
> >> don't want the fallback, and I can imagine contexts in which it could
> >> be a security problem.
> >
> > While I certainly would prefer the non-fallback behavior for init as
> > well, standard kernel practice has typically been to use "default y" for
> > previously built-in features that become configurable. And I'd
> > certainly prefer a compile-time configuration option like this (even
> > with default y) over a "strictinit" kernel command-line option.
>
> Fair enough.
>
> So: "default y" for a release or two, then switch the default?

Default y for a few releases, get it on the radar of various
distributions so they make an explicit decision about it, then consider
changing the upstream default.

- Josh Triplett

2014-10-14 21:00:54

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:

> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
> > On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
> >> I significantly prefer default N. Scripts that play with init= really
> >> don't want the fallback, and I can imagine contexts in which it could
> >> be a security problem.
> >
> > While I certainly would prefer the non-fallback behavior for init as
> > well, standard kernel practice has typically been to use "default y" for
> > previously built-in features that become configurable. And I'd
> > certainly prefer a compile-time configuration option like this (even
> > with default y) over a "strictinit" kernel command-line option.
> >
>
> Fair enough.
>
> So: "default y" for a release or two, then switch the default? Having
> default y will annoy virtme, though it's not the end of the world.
> Virtme is intended to work with more-or-less-normal kernels.
>

Adding another Kconfig option is tiresome. What was wrong with strictinit=?

2014-10-14 21:21:32

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Tue, Oct 14, 2014 at 2:00 PM, Andrew Morton
<[email protected]> wrote:
> On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:
>
>> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
>> > On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
>> >> I significantly prefer default N. Scripts that play with init= really
>> >> don't want the fallback, and I can imagine contexts in which it could
>> >> be a security problem.
>> >
>> > While I certainly would prefer the non-fallback behavior for init as
>> > well, standard kernel practice has typically been to use "default y" for
>> > previously built-in features that become configurable. And I'd
>> > certainly prefer a compile-time configuration option like this (even
>> > with default y) over a "strictinit" kernel command-line option.
>> >
>>
>> Fair enough.
>>
>> So: "default y" for a release or two, then switch the default? Having
>> default y will annoy virtme, though it's not the end of the world.
>> Virtme is intended to work with more-or-less-normal kernels.
>>
>
> Adding another Kconfig option is tiresome. What was wrong with strictinit=?

The consensus seems to be that adding a non-default option to get
sensible behavior would be unfortunate. Also, I don't like
strictinit=, since backwards-compatible setups will have to do
init=foo strictinit=foo. My original proposal was init=foo
strictinit.

TBH, my preference would be to make strict mode unconditional.
http://xkcd.com/1172/

--Andy

--
Andy Lutomirski
AMA Capital Management, LLC

2014-10-15 05:46:18

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On 10/14/2014 2:21 PM, Andy Lutomirski wrote:
> On Tue, Oct 14, 2014 at 2:00 PM, Andrew Morton
> <[email protected]> wrote:
>> On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:
>>
>>> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
>>>> On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
>>>>> I significantly prefer default N. Scripts that play with init= really
>>>>> don't want the fallback, and I can imagine contexts in which it could
>>>>> be a security problem.
>>>>
>>>> While I certainly would prefer the non-fallback behavior for init as
>>>> well, standard kernel practice has typically been to use "default y" for
>>>> previously built-in features that become configurable. And I'd
>>>> certainly prefer a compile-time configuration option like this (even
>>>> with default y) over a "strictinit" kernel command-line option.
>>>>
>>>
>>> Fair enough.
>>>
>>> So: "default y" for a release or two, then switch the default? Having
>>> default y will annoy virtme, though it's not the end of the world.
>>> Virtme is intended to work with more-or-less-normal kernels.
>>>
>>
>> Adding another Kconfig option is tiresome. What was wrong with strictinit=?
>
> The consensus seems to be that adding a non-default option to get

^^^^^^^^^ I do not think you know what the word consensus means. :-)

I did not agree.

I do agree with Andrew (but with no opinion on whether "strictinit=SOMETHING"
or just "strictinit".

> sensible behavior would be unfortunate. Also, I don't like
^^^^^^^^^^^^^^^^^
behavior that is useful in some or many contexts

> strictinit=, since backwards-compatible setups will have to do
> init=foo strictinit=foo. My original proposal was init=foo
> strictinit.
>
> TBH, my preference would be to make strict mode unconditional.
> http://xkcd.com/1172/
>
> --Andy
>

2014-10-15 05:57:05

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Tue, Oct 14, 2014 at 10:46 PM, Frank Rowand <[email protected]> wrote:
> On 10/14/2014 2:21 PM, Andy Lutomirski wrote:
>> On Tue, Oct 14, 2014 at 2:00 PM, Andrew Morton
>> <[email protected]> wrote:
>>> On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:
>>>
>>>> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
>>>>> On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
>>>>>> I significantly prefer default N. Scripts that play with init= really
>>>>>> don't want the fallback, and I can imagine contexts in which it could
>>>>>> be a security problem.
>>>>>
>>>>> While I certainly would prefer the non-fallback behavior for init as
>>>>> well, standard kernel practice has typically been to use "default y" for
>>>>> previously built-in features that become configurable. And I'd
>>>>> certainly prefer a compile-time configuration option like this (even
>>>>> with default y) over a "strictinit" kernel command-line option.
>>>>>
>>>>
>>>> Fair enough.
>>>>
>>>> So: "default y" for a release or two, then switch the default? Having
>>>> default y will annoy virtme, though it's not the end of the world.
>>>> Virtme is intended to work with more-or-less-normal kernels.
>>>>
>>>
>>> Adding another Kconfig option is tiresome. What was wrong with strictinit=?
>>
>> The consensus seems to be that adding a non-default option to get
>
> ^^^^^^^^^ I do not think you know what the word consensus means. :-)
>
> I did not agree.
>
> I do agree with Andrew (but with no opinion on whether "strictinit=SOMETHING"
> or just "strictinit".
>
>> sensible behavior would be unfortunate. Also, I don't like

Even you're not disagreeing that it's ugly, though, FWIW.

> ^^^^^^^^^^^^^^^^^
> behavior that is useful in some or many contexts

Is there a context in which the current behavior is useful beyond
"whoops, I typoed my grub command line edit, and I still want my
system to boot into *something* even if it's the wrong thing"? I'm
not personally that sympathetic to that particular use case, but maybe
there's another one.

--Andy

>
>> strictinit=, since backwards-compatible setups will have to do
>> init=foo strictinit=foo. My original proposal was init=foo
>> strictinit.
>>
>> TBH, my preference would be to make strict mode unconditional.
>> http://xkcd.com/1172/
>>
>> --Andy
>>
>



--
Andy Lutomirski
AMA Capital Management, LLC

2014-10-15 06:37:33

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On 10/14/2014 10:56 PM, Andy Lutomirski wrote:
> On Tue, Oct 14, 2014 at 10:46 PM, Frank Rowand <[email protected]> wrote:
>> On 10/14/2014 2:21 PM, Andy Lutomirski wrote:
>>> On Tue, Oct 14, 2014 at 2:00 PM, Andrew Morton
>>> <[email protected]> wrote:
>>>> On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:
>>>>
>>>>> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
>>>>>> On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
>>>>>>> I significantly prefer default N. Scripts that play with init= really
>>>>>>> don't want the fallback, and I can imagine contexts in which it could
>>>>>>> be a security problem.
>>>>>>
>>>>>> While I certainly would prefer the non-fallback behavior for init as
>>>>>> well, standard kernel practice has typically been to use "default y" for
>>>>>> previously built-in features that become configurable. And I'd
>>>>>> certainly prefer a compile-time configuration option like this (even
>>>>>> with default y) over a "strictinit" kernel command-line option.
>>>>>>
>>>>>
>>>>> Fair enough.
>>>>>
>>>>> So: "default y" for a release or two, then switch the default? Having
>>>>> default y will annoy virtme, though it's not the end of the world.
>>>>> Virtme is intended to work with more-or-less-normal kernels.
>>>>>
>>>>
>>>> Adding another Kconfig option is tiresome. What was wrong with strictinit=?
>>>
>>> The consensus seems to be that adding a non-default option to get
>>
>> ^^^^^^^^^ I do not think you know what the word consensus means. :-)
>>
>> I did not agree.
>>
>> I do agree with Andrew (but with no opinion on whether "strictinit=SOMETHING"
>> or just "strictinit".
>>
>>> sensible behavior would be unfortunate. Also, I don't like
>
> Even you're not disagreeing that it's ugly, though, FWIW.

You are putting (lack of) words in my mouth. I did not comment on
"ugly" because it did not seem that big a deal to me. I have no
desire to bikeshed on ugly in this particular instance.

>
>> ^^^^^^^^^^^^^^^^^
>> behavior that is useful in some or many contexts
>
> Is there a context in which the current behavior is useful beyond
> "whoops, I typoed my grub command line edit, and I still want my
> system to boot into *something* even if it's the wrong thing"? I'm
> not personally that sympathetic to that particular use case, but maybe
> there's another one.

We've been through this before. I should have ignored your "sensible
behavior" comment. Sorry, again no need for me to bike shed on that.

The question from Andrew was whether to use a config option or a command
line option. One could choose either behavior as default, whether
controlled by command line or config option.


>
> --Andy
>
>>
>>> strictinit=, since backwards-compatible setups will have to do
>>> init=foo strictinit=foo. My original proposal was init=foo
>>> strictinit.
>>>
>>> TBH, my preference would be to make strict mode unconditional.
>>> http://xkcd.com/1172/
>>>
>>> --Andy

2014-10-15 15:19:07

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On 10/14/14 16:00, Andrew Morton wrote:
> On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:
>
>> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
>>> On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
>>>> I significantly prefer default N. Scripts that play with init= really
>>>> don't want the fallback, and I can imagine contexts in which it could
>>>> be a security problem.
>>>
>>> While I certainly would prefer the non-fallback behavior for init as
>>> well, standard kernel practice has typically been to use "default y" for
>>> previously built-in features that become configurable. And I'd
>>> certainly prefer a compile-time configuration option like this (even
>>> with default y) over a "strictinit" kernel command-line option.
>>>
>>
>> Fair enough.
>>
>> So: "default y" for a release or two, then switch the default? Having
>> default y will annoy virtme, though it's not the end of the world.
>> Virtme is intended to work with more-or-less-normal kernels.
>>
>
> Adding another Kconfig option is tiresome. What was wrong with strictinit=?

Adding kernel command line options isn't tiresome? A quick grep of
kernel-parameters.txt ballparks us at around 600 of them so far, and the
one you're proposing one would would translate to "and I really mean it".

Chopping out legacy code with an ifconfig is a step to deprecating it
and eventually removing it. Adding code to do less is bloat that will
stay there forever.

The old behavior is surprising, most people putting together systems in
the past 10 years probably aren't aware it does that unless they got bit
by it. You can't specify a series of fallback inits from the command
line, only the magic hardcoded values can provide a random mix of places
to look for "init" (including in /etc/init for some reason? Has that
_ever_ been a thing?) and then calling a different program entirely
("sh") at a hardwired absolute path because reasons.

Initramfs does not do this fallback nonsense, it has one place it looks
("/init") and rdinit= changes that without magic fallbacks to the old
one if it's not found. If you specify rdinit= it won't even look for
"/init":

if (!ramdisk_execute_command)
ramdisk_execute_command = "/init";

if (sys_access((const char __user *) ramdisk_execute_command, 0)
!= 0) {
ramdisk_execute_command = NULL;
prepare_namespace();
}


(I.E. if it finds the one and only rdinit command name in ramfs, it
doesn't overmount it with the root= contents. Once overmounted, any
other init programs in ramfs wouldn't matter.)

The current behavior is inconsistent and crazy, and only there for
legacy reasons. We _already_ don't do it for newer codepaths. That's why
chopping it out with kconfig (and immediately deprecating the config
option) makes more sense than adding extra code to not do it.

Rob

2014-10-20 20:15:20

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Tue, Oct 14, 2014 at 2:00 PM, Andrew Morton
<[email protected]> wrote:
> On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:
>
>> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
>> > On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
>> >> I significantly prefer default N. Scripts that play with init= really
>> >> don't want the fallback, and I can imagine contexts in which it could
>> >> be a security problem.
>> >
>> > While I certainly would prefer the non-fallback behavior for init as
>> > well, standard kernel practice has typically been to use "default y" for
>> > previously built-in features that become configurable. And I'd
>> > certainly prefer a compile-time configuration option like this (even
>> > with default y) over a "strictinit" kernel command-line option.
>> >
>>
>> Fair enough.
>>
>> So: "default y" for a release or two, then switch the default? Having
>> default y will annoy virtme, though it's not the end of the world.
>> Virtme is intended to work with more-or-less-normal kernels.
>>
>
> Adding another Kconfig option is tiresome. What was wrong with strictinit=?

Now that this thread has gotten absurdly wrong, any thoughts?

My preference order is:

1. The patch as is.
2. The patch, minus the config option (i.e. making it unconditional).
3. Something else.

I would very much prefer to get *something* merged. The current
behavior is problematic for scripted kernel boots that don't use
initramfs.

I can be flexible on the something else. One option would be to allow
a whole list of commands in init=, but that has compatibility issues.
Another would be adding an option like init_fallback=/bin/sh. A third
is the original strictinit mechanism. I don't really like any of
them, because they're all more complex.

IOW, the no-fallback behavior is easy to implement, easy to
understand, and has extremely predictable behavior. The fallback
behavior is more user friendly if you consider having a chance of
booting to something useful if you typo your init= option (but also a
chance of booting to something actively undesirable).

--Andy

--
Andy Lutomirski
AMA Capital Management, LLC

2014-10-20 21:02:07

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Mon, Oct 20, 2014 at 01:14:54PM -0700, Andy Lutomirski wrote:
> On Tue, Oct 14, 2014 at 2:00 PM, Andrew Morton
> <[email protected]> wrote:
> > On Wed, 1 Oct 2014 11:13:14 -0700 Andy Lutomirski <[email protected]> wrote:
> >
> >> On Wed, Oct 1, 2014 at 11:05 AM, <[email protected]> wrote:
> >> > On Tue, Sep 30, 2014 at 09:53:56PM -0700, Andy Lutomirski wrote:
> >> >> I significantly prefer default N. Scripts that play with init= really
> >> >> don't want the fallback, and I can imagine contexts in which it could
> >> >> be a security problem.
> >> >
> >> > While I certainly would prefer the non-fallback behavior for init as
> >> > well, standard kernel practice has typically been to use "default y" for
> >> > previously built-in features that become configurable. And I'd
> >> > certainly prefer a compile-time configuration option like this (even
> >> > with default y) over a "strictinit" kernel command-line option.
> >> >
> >>
> >> Fair enough.
> >>
> >> So: "default y" for a release or two, then switch the default? Having
> >> default y will annoy virtme, though it's not the end of the world.
> >> Virtme is intended to work with more-or-less-normal kernels.
> >>
> >
> > Adding another Kconfig option is tiresome. What was wrong with strictinit=?
>
> Now that this thread has gotten absurdly wrong, any thoughts?
>
> My preference order is:
>
> 1. The patch as is.
> 2. The patch, minus the config option (i.e. making it unconditional).
> 3. Something else.

Agreed.

> I would very much prefer to get *something* merged. The current
> behavior is problematic for scripted kernel boots that don't use
> initramfs.
>
> I can be flexible on the something else. One option would be to allow
> a whole list of commands in init=, but that has compatibility issues.
> Another would be adding an option like init_fallback=/bin/sh. A third
> is the original strictinit mechanism. I don't really like any of
> them, because they're all more complex.

I agree, particularly because they *add* more logic rather than
*removing* logic. In this case, I think a Kconfig option makes sense,
because it controls additional behavior (the init fallback mechanism).

Plus, adding more code to control init fallback at runtime then means I
have more code to compile out to make the kernel smaller, so I'd end up
adding that Kconfig option anyway. :)

> IOW, the no-fallback behavior is easy to implement, easy to
> understand, and has extremely predictable behavior. The fallback
> behavior is more user friendly if you consider having a chance of
> booting to something useful if you typo your init= option (but also a
> chance of booting to something actively undesirable).

Here's an alternative proposal: how about we change the default
*without* a Kconfig option, see if anyone screams, and if they do, we
add that code back in under a Kconfig option as in your current patch?

Would that make your Kconfig senses stop tingling, Andrew? :)

- Josh Triplett

2014-10-20 21:28:39

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Mon, 20 Oct 2014 14:01:55 -0700 Josh Triplett <[email protected]> wrote:

> > IOW, the no-fallback behavior is easy to implement, easy to
> > understand, and has extremely predictable behavior. The fallback
> > behavior is more user friendly if you consider having a chance of
> > booting to something useful if you typo your init= option (but also a
> > chance of booting to something actively undesirable).
>
> Here's an alternative proposal: how about we change the default
> *without* a Kconfig option, see if anyone screams, and if they do, we
> add that code back in under a Kconfig option as in your current patch?
>
> Would that make your Kconfig senses stop tingling, Andrew? :)

Mumble. I suppose we can run with it as-is: at least the config option
is there to allow people to repair any damage easily.

However we don't have any way of remembering to remove the config
option later coz someone removed feature-removal-schedule.txt, which
was a useful feature.

2014-10-20 21:34:38

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Mon, Oct 20, 2014 at 2:28 PM, Andrew Morton
<[email protected]> wrote:
> On Mon, 20 Oct 2014 14:01:55 -0700 Josh Triplett <[email protected]> wrote:
>
>> > IOW, the no-fallback behavior is easy to implement, easy to
>> > understand, and has extremely predictable behavior. The fallback
>> > behavior is more user friendly if you consider having a chance of
>> > booting to something useful if you typo your init= option (but also a
>> > chance of booting to something actively undesirable).
>>
>> Here's an alternative proposal: how about we change the default
>> *without* a Kconfig option, see if anyone screams, and if they do, we
>> add that code back in under a Kconfig option as in your current patch?
>>
>> Would that make your Kconfig senses stop tingling, Andrew? :)
>
> Mumble. I suppose we can run with it as-is: at least the config option
> is there to allow people to repair any damage easily.
>
> However we don't have any way of remembering to remove the config
> option later coz someone removed feature-removal-schedule.txt, which
> was a useful feature.

Does -mm have a next+1 section? If so, you could queue it up now :)

--Andy

2014-10-20 21:41:23

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Mon, 20 Oct 2014 14:34:14 -0700 Andy Lutomirski <[email protected]> wrote:

> On Mon, Oct 20, 2014 at 2:28 PM, Andrew Morton
> <[email protected]> wrote:
> > On Mon, 20 Oct 2014 14:01:55 -0700 Josh Triplett <[email protected]> wrote:
> >
> >> > IOW, the no-fallback behavior is easy to implement, easy to
> >> > understand, and has extremely predictable behavior. The fallback
> >> > behavior is more user friendly if you consider having a chance of
> >> > booting to something useful if you typo your init= option (but also a
> >> > chance of booting to something actively undesirable).
> >>
> >> Here's an alternative proposal: how about we change the default
> >> *without* a Kconfig option, see if anyone screams, and if they do, we
> >> add that code back in under a Kconfig option as in your current patch?
> >>
> >> Would that make your Kconfig senses stop tingling, Andrew? :)
> >
> > Mumble. I suppose we can run with it as-is: at least the config option
> > is there to allow people to repair any damage easily.
> >
> > However we don't have any way of remembering to remove the config
> > option later coz someone removed feature-removal-schedule.txt, which
> > was a useful feature.
>
> Does -mm have a next+1 section? If so, you could queue it up now :)

Yes, I can do that. I add little notes-to-self in the series file to
remember such things.

2014-10-20 21:42:30

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Mon, Oct 20, 2014 at 2:41 PM, Andrew Morton
<[email protected]> wrote:
> On Mon, 20 Oct 2014 14:34:14 -0700 Andy Lutomirski <[email protected]> wrote:
>
>> On Mon, Oct 20, 2014 at 2:28 PM, Andrew Morton
>> <[email protected]> wrote:
>> > On Mon, 20 Oct 2014 14:01:55 -0700 Josh Triplett <[email protected]> wrote:
>> >
>> >> > IOW, the no-fallback behavior is easy to implement, easy to
>> >> > understand, and has extremely predictable behavior. The fallback
>> >> > behavior is more user friendly if you consider having a chance of
>> >> > booting to something useful if you typo your init= option (but also a
>> >> > chance of booting to something actively undesirable).
>> >>
>> >> Here's an alternative proposal: how about we change the default
>> >> *without* a Kconfig option, see if anyone screams, and if they do, we
>> >> add that code back in under a Kconfig option as in your current patch?
>> >>
>> >> Would that make your Kconfig senses stop tingling, Andrew? :)
>> >
>> > Mumble. I suppose we can run with it as-is: at least the config option
>> > is there to allow people to repair any damage easily.
>> >
>> > However we don't have any way of remembering to remove the config
>> > option later coz someone removed feature-removal-schedule.txt, which
>> > was a useful feature.
>>
>> Does -mm have a next+1 section? If so, you could queue it up now :)
>
> Yes, I can do that. I add little notes-to-self in the series file to
> remember such things.

Should I send you a patch, or do you want to write it yourself?

--Andy

--
Andy Lutomirski
AMA Capital Management, LLC

2014-10-20 21:44:50

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v5] init: Disable defaults if init= fails

On Mon, 20 Oct 2014 14:42:07 -0700 Andy Lutomirski <[email protected]> wrote:

> >> Does -mm have a next+1 section? If so, you could queue it up now :)
> >
> > Yes, I can do that. I add little notes-to-self in the series file to
> > remember such things.
>
> Should I send you a patch, or do you want to write it yourself?

I hate not having anyone to blame.

2014-10-20 22:04:44

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH] init: Remove CONFIG_INIT_FALLBACK

CONFIG_INIT_FALLBACK adds config bloat without an obvious use case
that makes it worth keeping around. Delete it.

Signed-off-by: Andy Lutomirski <[email protected]>
---

Bring on the blame :)

init/Kconfig | 16 ----------------
init/main.c | 5 -----
2 files changed, 21 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index ebbd5846478e..e84c6423a2e5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1299,22 +1299,6 @@ source "usr/Kconfig"

endif

-config INIT_FALLBACK
- bool "Fall back to defaults if init= parameter is bad"
- default y
- help
- If enabled, the kernel will try the default init binaries if an
- explicit request from the init= parameter fails.
-
- This can have unexpected effects. For example, booting
- with init=/sbin/kiosk_app will run /sbin/init or even /bin/sh
- if /sbin/kiosk_app cannot be executed.
-
- The default value of Y is consistent with historical behavior.
- Selecting N is likely to be more appropriate for most uses,
- especially on kiosks and on kernels that are indended to be
- run under the control of a script.
-
config CC_OPTIMIZE_FOR_SIZE
bool "Optimize for size"
help
diff --git a/init/main.c b/init/main.c
index 2bd6105e5dc5..39dd52a3b78a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -960,13 +960,8 @@ static int __ref kernel_init(void *unused)
ret = run_init_process(execute_command);
if (!ret)
return 0;
-#ifndef CONFIG_INIT_FALLBACK
panic("Requested init %s failed (error %d).",
execute_command, ret);
-#else
- pr_err("Failed to execute %s (error %d). Attempting defaults...\n",
- execute_command, ret);
-#endif
}
if (!try_to_run_init_process("/sbin/init") ||
!try_to_run_init_process("/etc/init") ||
--
1.9.3

2014-10-20 22:06:20

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH] init: Remove CONFIG_INIT_FALLBACK

On Mon, Oct 20, 2014 at 03:04:36PM -0700, Andy Lutomirski wrote:
> CONFIG_INIT_FALLBACK adds config bloat without an obvious use case
> that makes it worth keeping around. Delete it.
>
> Signed-off-by: Andy Lutomirski <[email protected]>
> ---
>
> Bring on the blame :)

Reviewed-by: Josh Triplett <[email protected]>
Blame is easier to bear when shared. :)

> init/Kconfig | 16 ----------------
> init/main.c | 5 -----
> 2 files changed, 21 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index ebbd5846478e..e84c6423a2e5 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1299,22 +1299,6 @@ source "usr/Kconfig"
>
> endif
>
> -config INIT_FALLBACK
> - bool "Fall back to defaults if init= parameter is bad"
> - default y
> - help
> - If enabled, the kernel will try the default init binaries if an
> - explicit request from the init= parameter fails.
> -
> - This can have unexpected effects. For example, booting
> - with init=/sbin/kiosk_app will run /sbin/init or even /bin/sh
> - if /sbin/kiosk_app cannot be executed.
> -
> - The default value of Y is consistent with historical behavior.
> - Selecting N is likely to be more appropriate for most uses,
> - especially on kiosks and on kernels that are indended to be
> - run under the control of a script.
> -
> config CC_OPTIMIZE_FOR_SIZE
> bool "Optimize for size"
> help
> diff --git a/init/main.c b/init/main.c
> index 2bd6105e5dc5..39dd52a3b78a 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -960,13 +960,8 @@ static int __ref kernel_init(void *unused)
> ret = run_init_process(execute_command);
> if (!ret)
> return 0;
> -#ifndef CONFIG_INIT_FALLBACK
> panic("Requested init %s failed (error %d).",
> execute_command, ret);
> -#else
> - pr_err("Failed to execute %s (error %d). Attempting defaults...\n",
> - execute_command, ret);
> -#endif
> }
> if (!try_to_run_init_process("/sbin/init") ||
> !try_to_run_init_process("/etc/init") ||
> --
> 1.9.3
>

2014-10-21 03:45:59

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH] init: Remove CONFIG_INIT_FALLBACK

On 10/20/14 17:04, Andy Lutomirski wrote:
> --- a/init/main.c
> +++ b/init/main.c
> @@ -960,13 +960,8 @@ static int __ref kernel_init(void *unused)
> ret = run_init_process(execute_command);
> if (!ret)
> return 0;
> -#ifndef CONFIG_INIT_FALLBACK
> panic("Requested init %s failed (error %d).",
> execute_command, ret);
> -#else
> - pr_err("Failed to execute %s (error %d). Attempting defaults...\n",
> - execute_command, ret);
> -#endif
> }
> if (!try_to_run_init_process("/sbin/init") ||
> !try_to_run_init_process("/etc/init") ||
>

Would you like to remove the try_to_run_init_process() stack of random
hardwired names that we can never reach if we panic, or do you just want
to remove the error message?

Confused,

Rob

2014-10-21 04:02:36

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] init: Remove CONFIG_INIT_FALLBACK

On Mon, Oct 20, 2014 at 8:45 PM, Rob Landley <[email protected]> wrote:
> On 10/20/14 17:04, Andy Lutomirski wrote:
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -960,13 +960,8 @@ static int __ref kernel_init(void *unused)
>> ret = run_init_process(execute_command);
>> if (!ret)
>> return 0;
>> -#ifndef CONFIG_INIT_FALLBACK
>> panic("Requested init %s failed (error %d).",
>> execute_command, ret);
>> -#else
>> - pr_err("Failed to execute %s (error %d). Attempting defaults...\n",
>> - execute_command, ret);
>> -#endif
>> }
>> if (!try_to_run_init_process("/sbin/init") ||
>> !try_to_run_init_process("/etc/init") ||
>>
>
> Would you like to remove the try_to_run_init_process() stack of random
> hardwired names that we can never reach if we panic, or do you just want
> to remove the error message?
>

I'm confused. That code is reachable if there's no initramfs and
init= is not specified.

--Andy

> Confused,
>
> Rob



--
Andy Lutomirski
AMA Capital Management, LLC

2014-10-21 04:15:33

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH] init: Remove CONFIG_INIT_FALLBACK



On 10/20/14 23:02, Andy Lutomirski wrote:
> On Mon, Oct 20, 2014 at 8:45 PM, Rob Landley <[email protected]> wrote:
>> On 10/20/14 17:04, Andy Lutomirski wrote:
>>> --- a/init/main.c
>>> +++ b/init/main.c
>>> @@ -960,13 +960,8 @@ static int __ref kernel_init(void *unused)
>>> ret = run_init_process(execute_command);
>>> if (!ret)
>>> return 0;
>>> -#ifndef CONFIG_INIT_FALLBACK
>>> panic("Requested init %s failed (error %d).",
>>> execute_command, ret);
>>> -#else
>>> - pr_err("Failed to execute %s (error %d). Attempting defaults...\n",
>>> - execute_command, ret);
>>> -#endif
>>> }
>>> if (!try_to_run_init_process("/sbin/init") ||
>>> !try_to_run_init_process("/etc/init") ||
>>>
>>
>> Would you like to remove the try_to_run_init_process() stack of random
>> hardwired names that we can never reach if we panic, or do you just want
>> to remove the error message?
>>
>
> I'm confused. That code is reachable if there's no initramfs and
> init= is not specified.

Ah, I thought the purpose of the original patch was to make init=
required, but if not then fine.

/etc/init is still crazy, though.

Rob

2014-10-21 09:53:08

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] init: Remove CONFIG_INIT_FALLBACK

On Tue, Oct 21, 2014 at 12:04 AM, Andy Lutomirski <[email protected]> wrote:
> CONFIG_INIT_FALLBACK adds config bloat without an obvious use case
> that makes it worth keeping around. Delete it.
>
> Signed-off-by: Andy Lutomirski <[email protected]>
> ---
>
> Bring on the blame :)
>
> init/Kconfig | 16 ----------------
> init/main.c | 5 -----
> 2 files changed, 21 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index ebbd5846478e..e84c6423a2e5 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1299,22 +1299,6 @@ source "usr/Kconfig"
>
> endif
>
> -config INIT_FALLBACK
> - bool "Fall back to defaults if init= parameter is bad"
> - default y
> - help

The above is not in mainline, nor in -next?
Seems it only exists in a patch series you sent yourself?

Confused...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2014-10-21 10:05:24

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH] init: Remove CONFIG_INIT_FALLBACK

On Tue, Oct 21, 2014 at 11:53:04AM +0200, Geert Uytterhoeven wrote:
> On Tue, Oct 21, 2014 at 12:04 AM, Andy Lutomirski <[email protected]> wrote:
> > CONFIG_INIT_FALLBACK adds config bloat without an obvious use case
> > that makes it worth keeping around. Delete it.
> >
> > Signed-off-by: Andy Lutomirski <[email protected]>
> > ---
> >
> > Bring on the blame :)
> >
> > init/Kconfig | 16 ----------------
> > init/main.c | 5 -----
> > 2 files changed, 21 deletions(-)
> >
> > diff --git a/init/Kconfig b/init/Kconfig
> > index ebbd5846478e..e84c6423a2e5 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1299,22 +1299,6 @@ source "usr/Kconfig"
> >
> > endif
> >
> > -config INIT_FALLBACK
> > - bool "Fall back to defaults if init= parameter is bad"
> > - default y
> > - help
>
> The above is not in mainline, nor in -next?
> Seems it only exists in a patch series you sent yourself?
>
> Confused...

See the previous thread. The previous patch adds this config option to
make the current default behavior optional. The removal patch then
removes the old default behavior. Andrew plans to push them in
successive kernels, to give more review time.

- Josh Triplett