Date: Fri, 29 Dec 2023 14:14:55 -0500
From: Stefan Berger <stefanb@...ux.ibm.com>
To: Rob Landley <rob@...dley.net>, Askar Safin <safinaskar@...il.com>
Cc: gregkh@...uxfoundation.org, initramfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        zohar@...ux.ibm.com
Subject: Re: [PATCH v3] rootfs: Fix support for rootfstype= when root= is
 given



On 12/29/23 13:35, Rob Landley wrote:
> On 12/29/23 10:39, Stefan Berger wrote:> On 12/21/23 17:58, Askar Safin wrote:
>>> Hi, Rob. And Stefan.
>>>
>>> First of all, this patch got to linux-next (
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=author&q=Stefan+Berger
>>> ), so it seems it soon will be in mainline.
>>>
>>> On Thu, Dec 21, 2023 at 12:24 PM Rob Landley <rob@...dley.net> wrote:
>>>> Can you build tmpfs on a nommu system? Last I checked the plumbing expects swap,
>>>> but it's been a while...
>>> Okay, I agree, let's not remove ramfs.
>>>
>>> Still, I don't like this (already applied) patch. init= and rdinit=
>>> are two different options,
> 
> Because they control two different things which are often used at the same time.
> (Debian has an initramfs that hands off to the final root filesystem, for
> example. Hence the initramfs-tools package that runs every time apt-get updates
> the kernel.)
> 
> So being able to specify rdinit= to intercept the ramfs layer or init= to
> intercept the root= layer made sense, because they did different things.
> 
> But the only reason to specify anything nontrivial for the initramfs
> _filesystem_ mount properties is because you intend to stay there. They don't
> get used together.
> 
>>> and this is good.
> 
> Eh, not really. Strange legacy decision we're now stuck with. The kernel only
> ever runs one init task per boot. If init= was _also_ checked to see which file
> to run out of initramfs (and the plumbing still justs silently fails and moves
> on if it's not found) then the debian script would have been forced to do INIT=
> or similar to override the overmounted root's init task separately from initrd's
> init task, making it clear a script (not the kernel) is making that decision.
> 
> But that would have been a user-visible change, and when initramfs was going in
> they were trying to avoid user-visible changes that would force sysadmins to
> learn new stuff because the plumbing changed out from under them. (Like the
> change you're proposing now would.)
> 
>>> So, I think we should
>>> have two different options. Analogously they should be rootfstype= and
>>> rdrootfstype=.
> 
> You can't have a root= type of initramfs or tmpfs. The specified values can't
> overlap. The plumbing I wrote responds to specific values but otherwise leaves
> it for later users.
> 
>>> https://salsa.debian.org/kernel-team/initramfs-tools/-/blob/cf964bfb4362019fd7fba1e839e403ff950dca8e/init#L103
>>>
>>> As you can see, this shell script parses /proc/cmdline and assumes
>>> that rootfstype= always applies to real root.
> 
> The script is running _in_ the initramfs, which is already loaded and running at
> that point. Meaning the _kernel_ will not parse root= at that point, userspace
> has to do it.
> 
>>> So, if someone sets
>>> rootfstype= to tmpfs or ramfs, this will likely break this script.
> 
> Which was the same 10 years ago?
> 
> The script is running in a context where initramfs is not persistent, so
> overriding it to be a tmpfs has no benefit. (I mean you _can_... Nobody does,
> because we're gonna switch_root off of it.)
> 
> And once code _is_ running in initramfs, the kernel's internal root= automounter
> will never run. The initramfs code can parse /proc/cmdline to use the same
> arguments as the kernel, or it could much more easily use the "any unrecognized
> arguments get set as environment variables in PID 1" and use ROOT= or similar,
> like many scripts do.
> 
> Modifying kernel code that NEVER RUNS in the case you're pointing out seems
> silly to me.
> 
> That said, the code I wrote is doing a strstr to see if the argument's there,
> but doesn't care what ELSE is there, so it could easily be
> "rootfstype=tmpfs,ext4" and have the userspace script also filter the argument
> for just what it's interested in, since at that point it's NOT THE KERNEL DOING IT.

It's a bit tricky that this particular option, that can support a 
comma-separated list, is shared between kernel and user space and user 
space does not already filter-out what is not relevant for it.

> 
>> Setting the kernel boot command line option rootfstype= to tmpfs or
>> ramfs was possible so far and that's what the documentation and code
>> supported so far as well. The bug surfaced when root= was provided, in
>> which case it was ignored.
> 
> No, as I explained when I wrote the initmpfs code in 2013 when you say root= you
> are explicitly requesting the kernel mount a second file system over rootfs

 From the perspective of needing xattr support in initramfs it's 
unfortunately not so obvious what the filesystem type of the kernel's 
rootfs (presumably the 1st file system) has to do with the option given 
for the 2nd filesystem. Though the Debian scripts are the bigger problem 
it seems. However, for those one could argue that the Debian scripts 
could be updated and for as long as they are not able to filter-out the 
tmpfs or ramfs options we are interested in one cannot pass these 
options or a comma-separated list on systems that run the current Debian 
scripts.

> (that's what root= MEANS), and thus don't bother making it a (more expensive)
> tmpfs because it's not sticking around.

That's true unless you want to use IMA signature enforcement in the 
initramfs already and tmpfs is now required.

    Stefan

Powered by blists - more mailing lists