linux-kernel - Re: [PATCH] initramfs: Always do fput() and load modules after rootfs populate

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 5 May 2017 16:43:14 +0900
From:   Stafford Horne <shorne@...il.com>
To:     Al Viro <viro@...IV.linux.org.uk>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Lokesh Vutla <lokeshvutla@...com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] initramfs: Always do fput() and load modules after
 rootfs populate

On Thu, May 04, 2017 at 07:41:48PM +0100, Al Viro wrote:
> On Thu, May 04, 2017 at 09:47:00PM +0900, Stafford Horne wrote:
> > In OpenRISC we do not have a bootloader passed initrd, but the built in
> > initramfs does contain the /init and other binaries, including modules.
> > The previous commit 08865514805d2 ("initramfs: finish fput() before
> > accessing any binary from initramfs") made a change to only call fput()
> > if the bootloader initrd was available, this caused intermittent crashes
> > for OpenRISC.
> > 
> > This patch changes the fput() to happen unconditionally if any rootfs is
> > loaded. Also, I added some comments to make it a bit more clear why we
> > call unpack_to_rootfs() multiple times.
> 
> Hmm...  Looks like there are two bugs: the older one (from "init, block: try
> to load default elevator module early during boot") that missed the case of
> said module being on built-in initramfs and then flush_delayed_fput() patch
> apparently using that as a marker...
> 
> Frankly, the life would be a lot more convenient if we had the whole
> populate_rootfs() run in userland.  It's using the normal syscalls anyway;
> the only place where it needs more is the access to initramfs images and
> that could be done e.g. by offering one or two file descriptors with
> splice() from those picking those images.  We could build a static binary,
> link _that_ into the kernel and turn the entire thing into "fork a thread,
> write that sucker to rootfs, synchronously close it and do kernel_execve",
> letting everything else (decompression, unpacking, etc.) done in there.
> 
> That was the original plan, actually, and that was the original motivation
> for klibc.  Unfortunately, klibc would have to live in the kernel tree for
> that to work, and that hadn't happened.  Pity, that...
> 
> Anyway, feel free to slap
> Acked-by: Al Viro <viro@...iv.linux.org.uk>
> on that patch.  I can take it through vfs.git, or either of you could send
> it directly to Linus - up to you.

Hi Al,

Thanks for the review and the background on this.  I could see this would
make it nicer/cleaner to have populate_rootfs() in userland.  But, I guess
if that means adding klibc and keeping this static binary all in tree the
benefits don't outweight the downside of having the extra code and changes.
Do you think it is work revisiting someday?

I have pushed to linus, lets see what he says, if anything.

-Stafford