Re: [PATCH v3] rootfs: Fix support for rootfstype= when root= is given

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxx> 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




[Index of Archives]     [Linux Kernel]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux