linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yury Norov <ynorov@caviumnetworks.com>
To: Rob Landley <rob@landley.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Prarit Bhargava <prarit@redhat.com>,
	Yang Shi <yang.shi@linaro.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Kees Cook <keescook@chromium.org>,
	Emese Revfy <re.emese@gmail.com>, Petr Mladek <pmladek@suse.com>,
	Fabian Frederick <fabf@skynet.be>
Subject: Re: Patch 0727d35de ("Make initramfs honor CONFIG_DEVTMPFS_MOUNT") breaks boot
Date: Wed, 24 May 2017 00:32:15 +0300	[thread overview]
Message-ID: <20170523213215.qmfnbf4bseqiwkky@yury-N73SV> (raw)
In-Reply-To: <66dd96ec-b170-0bf6-7746-5466270b3e15@landley.net>

On Tue, May 23, 2017 at 12:40:04PM -0500, Rob Landley wrote:
> On 05/23/2017 03:01 AM, Yury Norov wrote:
> > On Mon, May 22, 2017 at 09:07:54PM -0500, Rob Landley wrote:
> >> Your userspace mounted a tmpfs over /dev when it couldn't mount a second
> >> identical instance of devtmpfs over itself. If you had a static /dev in
> >> initramfs but didn't configure _in_ devtmpfs to your kernel, your broken
> >> error path would have taken that out too with a pointless tmpfs mount.
> > 
> > CONFIG_DEVTMPFS_MOUNT is enabled on my machine, so I think your
> > suggestion is correct. But I didn't do that specifically - I run
> > almost default kernel based on Ubuntu 14.04 config and environment.
> 
> I.E. ubuntu has a bug: they enabled CONFIG_DEVTMPFS_MOUNT and then
> launchd an initramfs instead (which didn't do the automount they
> requested so why request it), but if CONFIG_DEVTMPFS_MOUNT actually
> starts working in initramfs they have an insane error path that breaks
> the system, and does nothing _except_ break the system.
> 
> > Grepping the kernel code shows that arc, arm, arm64, m86k, metag, 
> > mips, nios2, openrisc, parisc, powerpc, sh, tile, um, x86 and xetensa
> > enable it by default.
> 
> Most of which Ubuntu doesn't support, so none of them could trigger the
> broken error path in ubuntu's init script.
> 
> Wait, are you saying you're doing a "make defconfig" on x86-64 and
> booting ubuntu from the result? (Or is this arm?) Is _that_ the config
> you still haven't specified in this conversation? I thought you were
> using the /boot/config-4.4.0-78-generic and friends ubuntu installs.
> (Which yes, also switch this symbol on.)

No, I'm not saying it. I run arm64 with the config attached below.

> I can add a "default n" line to drivers/base/Kconfig if "make defconfig"
> is what you're building from. (This symbol never specified a default in
> the first place, so I dunno which way it falls, but it's repeated in a
> gazillion defconfig files and not present in others... meaning I still
> dunno which way the default goes. When I do a "make defconfig" it uses
> arch/x86/configs/x86_64_defconfig because the kernel has multiple
> codepaths to accomplish the same thing. I'm not sure the built-in
> "default y" lines are used at all anymore? What a mess...)
> 
> But again, I'm just guessing what config you're using because you still
> haven't _said_. I'm still trying to guess what you're doing when you hit
> Ubuntu's bug.
> 
> > So it means for me that (at least) users why run
> > Ubuntu 14.04 will have bricked system one day after updating the
> > kernel.
> 
> Unless when they build their new kernel they open up menuconfig and
> switch this symbol off. Which can't be done because...?
> 
> Or you could add the devtmpfs.mount=0 argument to your kernel command
> line, as documented in the CONFIG_DEVTMPFS_MOUNT menuconfig help text.
> 
> The kernel already provides multiple workarounds for Ubuntu's bug, and
> the issue only hits people who are manually building a new kernel from
> source. If ubuntu provides a new kernel, I assume they'll tweak their
> config _and_ fix their initramfs error path (which is just plain wrong).
> 
> > If you say that currently CONFIG_DEVTMPFS_MOUNT is ignored by kernel,
> 
> It's not _me_ saying it, it's the kernel doing it. The patch is
> conceptually a straightforward fix on the kernel side to make it _not_
> ignore that symbol in that context.
> 
> > I think you cannot relay on it anymore because people may have it
> > enabled or disabled randomly.
> 
> I expected configs would have it randomly set, but the bug here is a
> broken error path that does something actively harmful rather than going
> "oh, we got a static /dev from somewhere, let's just leave it alone".
> This error path goes out of its way to blank the contents of /dev by
> mounting an empty tmpfs over it and leaving it empty, and then
> complaining that /dev is blank _because_it_blanked_it_.
> 
> If Ubuntu meant to intentionally halt the proceedings the script could
> have done that explicitly. What did the author of that error path think
> would happen, exactly?
> 
> > So the proper way is to remove broken
> > config option and introduce new one. BTW, I see it is used once in
> > drivers/base/devtmpfs.c.
> 
> How does removing the broken config option (or renaming it to
> CONFIG_DEFTMPFS_UBUNTU_IS_BROKEN) _not_ impact systems that were
> previously happily using it in the contexts where it already worked?
> 
> If it's too much to ask people to switch it off when it was previously
> on (but shouldn't have been), how is asking them to manually switch it
> back on when it was previously on and needs to stay on better? (And if
> you arrange it so "make oldconfig" migrates the old symbol to the new
> one automatically, how would that work around the broken error path in
> ubuntu's initramfs script? The rename becomes a NOP.)
> 
> If you're saying it should default to "n" I can send a patch. If you
> want me to tweak every arch/*/configs file that redundantly includes the
> same darn symbol, I can do that too. (Makes the patch big but it's just
> a sed invocation to do it.)
> 
> >> By the way, _why_ are you mounting a tmpfs over /dev on _initramfs_?
> >> That can already be tmpfs. (Commits 137fdcc18a59 through 6e19eded3684.)
> > 
> > Have no idea. This is how default Ubuntu works.
> 
> Can you switch this symbol off in your kernel config and see if it boots
> then? (Or are you using some sort of ubuntu kernel build tool that can
> build -rc2 but not let you change config symbols? I still have no idea
> how you're building this because you haven't said.)
> 
> I don't know how the kernel higher-ups want to deal with this because
> it's more a political issue than a technical one. Enabling
> DEVTMPFS_MOUNT for initramfs triggers a pair of bugs in Ubuntu: their
> config was wrong and their error checking was wrong. Fixing _either_
> would let the system boot, and there's also the command line workaround.
> The issue only affects people who manually install new kernels on an old
> system using the old (broken) config verbatim, and the simplest fix is
> to flip the config symbol in menuconfig.
> 
> If you're building a defconfig the kernel supplies, we can edit that to
> work around Ubuntu's bug. If you're building from a config ubuntu
> supplies, _they_ have to edit it and then maybe bumping it a release
> until Ubuntu can fix its breakage sounds reasonable. But so does letting
> people manually adjust their configs when they manually upgrade the
> kernel outside the normal ubuntu kernel uprade process. (When open
> source breaks, you get to keep the pieces.)
> 
> > Yury
> 
> Rob

It was 2 years ago, but AFAIR I took the Ubuntu image here:
http://cdimage.ubuntu.com/ubuntu-base/releases/14.04.1/release/ubuntu-base-14.04.1-core-arm64.tar.gz

Kernel config is attached. I build the kernel with simple 'make'.

Yury

  reply	other threads:[~2017-05-23 21:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-22 12:05 Patch 0727d35de ("Make initramfs honor CONFIG_DEVTMPFS_MOUNT") breaks boot Yury Norov
2017-05-23  2:07 ` Rob Landley
2017-05-23  8:01   ` Yury Norov
2017-05-23 17:40     ` Rob Landley
2017-05-23 21:32       ` Yury Norov [this message]
2017-05-23 23:08         ` Yury Norov
2017-05-25  5:55           ` Rob Landley
2017-05-25  6:13       ` Michael Ellerman
2017-09-10 23:43         ` Rob Landley
2017-09-11 11:45           ` Petr Mladek
2017-09-12  0:42             ` Sergey Senozhatsky
2017-09-13  2:45             ` Rob Landley
     [not found] ` <CAP2kuOwkM7hr5nOtrRSRdXcPi2y3KHVf4yFdDN1JW1o_mWo0yw@mail.gmail.com>
2017-05-25 13:44   ` Leo Yan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170523213215.qmfnbf4bseqiwkky@yury-N73SV \
    --to=ynorov@caviumnetworks.com \
    --cc=akpm@linux-foundation.org \
    --cc=fabf@skynet.be \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pmladek@suse.com \
    --cc=prarit@redhat.com \
    --cc=re.emese@gmail.com \
    --cc=rob@landley.net \
    --cc=yang.shi@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).