Date: Sun, 31 Dec 2023 11:03:26 -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/30/23 12:08, Rob Landley wrote:
> On 12/29/23 13:14, Stefan Berger wrote:>> 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.
> 
> Debian's code sometimes has bugs, especially their initramfs stuff doesn't get a
> lot of scrutiny:
> 
> https://lkml.iu.edu/hypermail/linux/kernel/1705.2/05611.html
> https://lkml.iu.edu/hypermail/linux/kernel/1705.3/01182.html
> https://lkml.org/lkml/2017/9/13/651#:~:text=Debian's
> 
> But they're pretty good about fixing bugs pointed out to them:
> 
> https://salsa.debian.org/kernel-team/initramfs-tools/-/commit/49e4a0555f51
> 
> The kernel having more capabilities here than Debian's userspace does isn't new,
> it's what gives debian's userspace the opportunity to gain new capabilities.
> 
> Although in this case, the patch in question still isn't in lkml 5 years later
> because Debian development is much more responsive than linux-kernel:
> 
> https://lkml.iu.edu/hypermail/linux/kernel/2302.2/05597.html
> 
>>>> 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.
> 
> Ping Ben if initramfs-tools needs updating?
> 
> I've been following the initramfs xattr support threads forever:
> 
> https://lkml.iu.edu/hypermail/linux/kernel/2207.3/06939.html
> 
> I'm happy to add new format support to toybox cpio if anybody comes to an
> agreement on what that should be, but last time there was "as long as we're here
> 32 bit timestamps" and "sparse file support could be" and various bikeshedding...
> 
> Was there a new thread I didn't get cc'd on? The last I have is... July 2022 I
> think?
> 
>> 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.
> 
> You can argue that current userspace does not take full advantage of the
> existing kernel API, sure.
> 
>>> (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.
> 
> I agree that if you want to add a new requirement, you may need to modify userspace.
> 
> Let me see if I understand your problem: it sounds like debian's initramfs-tools
> overloads the root= and rootfstype= arguments parsed by the kernel to have a
> second meaning (the kernel uses them for one thing, you want to use them for
> something else, and there's currently a semantic gap between the two.)

My intention is to be able to pass rootfstype= to the kernel and have it 
interpreted correctly in the presence of root=, which currently does not 
work. User space tools that interpret the value of rootfstype= as if 
this option belonged to user space is not helpful, though it should be 
easy to teach the user space scripts to strip a leading 'tmpfs,' or 
'ramfs,' from the rootfstype value and let them interpret the rest.

> 
> You want to add a new capability requiring a new build dependency in the
> initramfs-tools package because it's doing new stuff, but there cannot be any
> OTHER changes made to initramfs-tools, so the kernel should change its existing
> semantics instead.

I haven't even thought of what would need to be added to Debian's 
initramfs-tools package since my primary goal was to enable tmpfs for 
the initramfs on OpenBMC where we then read the xattr values from a file 
and write them into the filesystem because cpio format doesn't carry 
them. Also, I didn't expect that any user space tools would try to 
handle a kernel command line option as if it was theirs.

> 
> You can't NOT provide root=, and you can't provide initramfstype=tmpfs...

I only know about rootfstype= ( 
https://github.com/torvalds/linux/blob/master/init/do_mounts.c#L128 ). 
If currently handling of rootfstype= in presence of root= is not 
considered a bug and we should introduce initramfstype= instead, we 
could do that. But doesn't this become a bit confusing if rootfstype= 
can be passed when root= is absent but then initramfstype= must be used 
when root= is present?

This is 'our' patch describing the issue: 
https://github.com/torvalds/linux/blob/master/init/do_mounts.c#L128

> either, and those are the two existing ways to tell rootfs to be tmpfs instead
> of ramfs. You'd like to add a third way to specify the same thing.

Do you have a link to initramfstype= handling in kernel code?


   Stefan
> 
> Do I have that right?
> 
>>      Stefan
> 
> Rob

Powered by blists - more mailing lists