linux-kernel - Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 17 May 2017 16:16:12 -0500
From:   Rob Landley <rob@...dley.net>
To:     Michael Ellerman <mpe@...erman.id.au>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Prarit Bhargava <prarit@...hat.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Yang Shi <yang.shi@...aro.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Kees Cook <keescook@...omium.org>,
        Emese Revfy <re.emese@...il.com>,
        Petr Mladek <pmladek@...e.com>,
        Fabian Frederick <fabf@...net.be>
Subject: Re: [PATCHv2] Make initramfs honor CONFIG_DEVTMPFS_MOUNT



On 05/16/2017 10:58 PM, Michael Ellerman wrote:
> Rob Landley <rob@...dley.net> writes:
> 
>> diff --git a/init/main.c b/init/main.c
>> index f866510..9ec09ff 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -1055,8 +1049,17 @@ static noinline void __init kernel_init_freeable(void)
>>  	if (sys_access((const char __user *) ramdisk_execute_command, 0) != 0) {
>>  		ramdisk_execute_command = NULL;
>>  		prepare_namespace();
>> +	} else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
>> +		sys_mkdir("/dev", 0755);
>> +		devtmpfs_mount("/dev");
>>  	}
>>  
>> +	/* Open the /dev/console on the rootfs, this should never fail */
>> +	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
> 
> Sorry to pile on,

The correct phrase is "bikeshed". (It's a verb now.)

> but while you're moving it do you want

_I_ don't, no. I intentionally moved it unmodified. If you want to
submit a patch on top of mine, be my guest.

> to update this fairly misleading comment.

Define "should". (I'll get the popcorn.)

> It definitely can fail, eg. if /dev/console doesn't exist, or if no
> console driver is registered.

/dev/console not existing in an initramfs created by pointing
CONFIG_INITRAMFS_SOURCE at a directory created by a normal user was
pretty much my initial motivation for poking at this area, yes.

That said, /dev/console should always exist. My patch was just finding a
different way for it to exist so the condition was satisfied. Meaning
the comment isn't exactly _wrong_, just really terse. Feel free to
submit a patch rephrasing it.

Rob