[Buildroot] [PATCH] fs/cpio/init: remout / as read-only as a first step

[Buildroot] [PATCH] fs/cpio/init: remout / as read-only as a first step

[Andrey Smirnov]

When /init is present on root file system kernel does not take into
accout kernel arguments such as "ro" and does not remount root as
read-only. So prior to this commit the system would continue booting
with "rw" root filesystem until corresponding line in /etc/fstab to
remount it as "ro" was processed.

Change the code to immediately remount / as read-only and rely on
/etc/fstab processing to remount it as "rw" if that is what's selected
in Buildroot configuration.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Hi everyone,

Here's a little bit of a backstory for this patch:

I have a PowerPC, systemd based system that is set up to build and boot
from a compressed CPIO image. The system is build using stock
Buildroot with a number of patches from
https://git.buildroot.org/~ymorin/git/buildroot/log/?h=yem/systemd-skeleton
on top of it (to fix bug 7892).

Running against systemd 232 (not reproducable with 233), I would
occasionally see "systemd-remount-fs.service" fail to start because it
would try to re-mount '/' as read-only and fail due to '/' being
busy (I never figured out exactly what process was writing
there). That particular failure, being not very critical on my setup,
would leave my system in a state where everything worked as before,
but root filesystem would be accessible for writing.

Since all of the above run contrary to my expectaions (I expected
rootfs to be read-only to begin with) I went onto reading kernel
source to see if all of this was because I somehow misconfigured my
kernel boot paramters. As I mentioned in commit message, doing so
revealed that, when "/init" is present in the system, kernel would
mount root as R/W and pass control onto "/init" without trying to
honor "ro" bootflag. So, as far I can tell, in setup like this, it is
the responsiblity of "/init" script/executable to set things up
"correctly".

Needless to say that applying the change made my original problem --
"systemd-remount-fs.service" failing -- not reproducable.

I am not sure if this is a geniunly good fix or if I missed something
blatantly obvious and all of the above is my fault, so please let me
know what you think.

Thanks,
Andrey Smirnov

 fs/cpio/init | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/cpio/init b/fs/cpio/init
index a275482..349f98b 100755
--- a/fs/cpio/init
+++ b/fs/cpio/init
@@ -1,5 +1,6 @@
 #!/bin/sh
 # devtmpfs does not get automounted for initramfs
+/bin/mount -o remount,ro /dev/root
 /bin/mount -t devtmpfs devtmpfs /dev
 exec 0</dev/console
 exec 1>/dev/console
-- 
2.9.4

[Buildroot] [PATCH] fs/cpio/init: remout / as read-only as a first step

[Peter Korsgaard]

>>>>> "Andrey" == Andrey Smirnov <andrew.smirnov@gmail.com> writes:

 > When /init is present on root file system kernel does not take into
 > accout kernel arguments such as "ro" and does not remount root as
 > read-only. So prior to this commit the system would continue booting
 > with "rw" root filesystem until corresponding line in /etc/fstab to
 > remount it as "ro" was processed.

 > Change the code to immediately remount / as read-only and rely on
 > /etc/fstab processing to remount it as "rw" if that is what's selected
 > in Buildroot configuration.

 > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

I see what you're getting at, but this doesn't actually work (with
busybox mount at least):

NET: Registered protocol family 17
Freeing unused kernel memory: 2528K
This architecture does not have kernel memory protection.
mount: can't read '/proc/mounts': No such file or directory
Starting logging: OK
Initializing random number generator... done.

The reason is that mount had to read the current mount flags (either
directly in /proc/mounts or through /etc/mtab depending on busybox
config, but we symlink /etc/mtab to /proc/mounts) to be able to remount,
and as /proc is not mounted in an initramfs this doesn't work.

We could conceptually mount /proc in /init as well, but then things gets
confusing when /sbin/init does it again.

I don't see any clean way of doing this without adding quite some
complexity, and I think your usecase (systemd in initramfs that you want
ro) is so special that I would suggest you just use a custom /init in
your rootfs overlay.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] fs/cpio/init: remout / as read-only as a first step

[Andrey Smirnov]

On Wed, Jul 5, 2017 at 7:17 AM, Peter Korsgaard <peter@korsgaard.com> wrote:
>>>>>> "Andrey" == Andrey Smirnov <andrew.smirnov@gmail.com> writes:
>
>  > When /init is present on root file system kernel does not take into
>  > accout kernel arguments such as "ro" and does not remount root as
>  > read-only. So prior to this commit the system would continue booting
>  > with "rw" root filesystem until corresponding line in /etc/fstab to
>  > remount it as "ro" was processed.
>
>  > Change the code to immediately remount / as read-only and rely on
>  > /etc/fstab processing to remount it as "rw" if that is what's selected
>  > in Buildroot configuration.
>
>  > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>
> I see what you're getting at, but this doesn't actually work (with
> busybox mount at least):
>
> NET: Registered protocol family 17
> Freeing unused kernel memory: 2528K
> This architecture does not have kernel memory protection.
> mount: can't read '/proc/mounts': No such file or directory
> Starting logging: OK
> Initializing random number generator... done.
>
> The reason is that mount had to read the current mount flags (either
> directly in /proc/mounts or through /etc/mtab depending on busybox
> config, but we symlink /etc/mtab to /proc/mounts) to be able to remount,
> and as /proc is not mounted in an initramfs this doesn't work.
>

Hmm, fascinating. I think the reason why I didn't see this and why it
worked for me is because systemd selects BR2_PACKAGE_UTIL_LINUX_MOUNT
and my system is using it instead of what busybox provides.

> We could conceptually mount /proc in /init as well, but then things gets
> confusing when /sbin/init does it again.
>
> I don't see any clean way of doing this without adding quite some
> complexity, and I think your usecase (systemd in initramfs that you want
> ro) is so special that I would suggest you just use a custom /init in
> your rootfs overlay.
>

What about either of the following:

- Adding that line and silencing errors, so that on on systems with
busybox's mount it would be effectively a no-op and systems that use
util-linux it would achieve desired results?

- Making BR2_TARGET_ROOTFS_CPIO select BR2_PACKAGE_UTIL_LINUX_MOUNT?

- Optionally adding that line if BR2_PACKAGE_UTIL_LINUX_MOUNT?

- Adding a note to fs/cpio/Config.in that root filesystem will become
read-only only after corresponding line in /etc/fstab is processed
successfully?

I am more than happy to alter the patch to accommodate for either of
the above or just drop it and keep this change private to my buildroot
customization layer.

Thanks,
Andrey Smirnov

[Buildroot] [PATCH] fs/cpio/init: remout / as read-only as a first step

[Arnout Vandecappelle]

On 05-07-17 22:52, Andrey Smirnov wrote:
> On Wed, Jul 5, 2017 at 7:17 AM, Peter Korsgaard <peter@korsgaard.com> wrote:
>>>>>>> "Andrey" == Andrey Smirnov <andrew.smirnov@gmail.com> writes:
>>
>>  > When /init is present on root file system kernel does not take into
>>  > accout kernel arguments such as "ro" and does not remount root as
>>  > read-only. So prior to this commit the system would continue booting
>>  > with "rw" root filesystem until corresponding line in /etc/fstab to
>>  > remount it as "ro" was processed.
>>
>>  > Change the code to immediately remount / as read-only and rely on
>>  > /etc/fstab processing to remount it as "rw" if that is what's selected
>>  > in Buildroot configuration.
>>
>>  > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>>
>> I see what you're getting at, but this doesn't actually work (with
>> busybox mount at least):
>>
>> NET: Registered protocol family 17
>> Freeing unused kernel memory: 2528K
>> This architecture does not have kernel memory protection.
>> mount: can't read '/proc/mounts': No such file or directory
>> Starting logging: OK
>> Initializing random number generator... done.
>>
>> The reason is that mount had to read the current mount flags (either
>> directly in /proc/mounts or through /etc/mtab depending on busybox
>> config, but we symlink /etc/mtab to /proc/mounts) to be able to remount,
>> and as /proc is not mounted in an initramfs this doesn't work.
>>
> 
> Hmm, fascinating. I think the reason why I didn't see this and why it
> worked for me is because systemd selects BR2_PACKAGE_UTIL_LINUX_MOUNT
> and my system is using it instead of what busybox provides.
> 
>> We could conceptually mount /proc in /init as well, but then things gets
>> confusing when /sbin/init does it again.
>>
>> I don't see any clean way of doing this without adding quite some
>> complexity, and I think your usecase (systemd in initramfs that you want
>> ro) is so special that I would suggest you just use a custom /init in
>> your rootfs overlay.
>>
> 
> What about either of the following:
> 
> - Adding that line and silencing errors, so that on on systems with
> busybox's mount it would be effectively a no-op and systems that use
> util-linux it would achieve desired results?
> 
> - Making BR2_TARGET_ROOTFS_CPIO select BR2_PACKAGE_UTIL_LINUX_MOUNT?
> 
> - Optionally adding that line if BR2_PACKAGE_UTIL_LINUX_MOUNT?

 All of this is fairly complicated for something that is really a corner case.


> - Adding a note to fs/cpio/Config.in that root filesystem will become
> read-only only after corresponding line in /etc/fstab is processed
> successfully?

 The thing is, depending on various circumstances, the kernel may start the
rootfs either rw or ro. It's pretty complicated. It's not even specific for
cpio/initramfs (except that in that case it always starts rw so it's in fact
simpler).

 You *might* add a comment like that in the BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
option, but even that is a bit too much detail I think.


> I am more than happy to alter the patch to accommodate for either of
> the above or just drop it and keep this change private to my buildroot
> customization layer.

 Yes, just put your version of /init in your fs-overlay and Bob's your uncle.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] fs/cpio/init: remout / as read-only as a first step

[Andrey Smirnov]

On Wed, Jul 5, 2017 at 3:18 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
> On 05-07-17 22:52, Andrey Smirnov wrote:
>> On Wed, Jul 5, 2017 at 7:17 AM, Peter Korsgaard <peter@korsgaard.com> wrote:
>>>>>>>> "Andrey" == Andrey Smirnov <andrew.smirnov@gmail.com> writes:
>>>
>>>  > When /init is present on root file system kernel does not take into
>>>  > accout kernel arguments such as "ro" and does not remount root as
>>>  > read-only. So prior to this commit the system would continue booting
>>>  > with "rw" root filesystem until corresponding line in /etc/fstab to
>>>  > remount it as "ro" was processed.
>>>
>>>  > Change the code to immediately remount / as read-only and rely on
>>>  > /etc/fstab processing to remount it as "rw" if that is what's selected
>>>  > in Buildroot configuration.
>>>
>>>  > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>>>
>>> I see what you're getting at, but this doesn't actually work (with
>>> busybox mount at least):
>>>
>>> NET: Registered protocol family 17
>>> Freeing unused kernel memory: 2528K
>>> This architecture does not have kernel memory protection.
>>> mount: can't read '/proc/mounts': No such file or directory
>>> Starting logging: OK
>>> Initializing random number generator... done.
>>>
>>> The reason is that mount had to read the current mount flags (either
>>> directly in /proc/mounts or through /etc/mtab depending on busybox
>>> config, but we symlink /etc/mtab to /proc/mounts) to be able to remount,
>>> and as /proc is not mounted in an initramfs this doesn't work.
>>>
>>
>> Hmm, fascinating. I think the reason why I didn't see this and why it
>> worked for me is because systemd selects BR2_PACKAGE_UTIL_LINUX_MOUNT
>> and my system is using it instead of what busybox provides.
>>
>>> We could conceptually mount /proc in /init as well, but then things gets
>>> confusing when /sbin/init does it again.
>>>
>>> I don't see any clean way of doing this without adding quite some
>>> complexity, and I think your usecase (systemd in initramfs that you want
>>> ro) is so special that I would suggest you just use a custom /init in
>>> your rootfs overlay.
>>>
>>
>> What about either of the following:
>>
>> - Adding that line and silencing errors, so that on on systems with
>> busybox's mount it would be effectively a no-op and systems that use
>> util-linux it would achieve desired results?
>>
>> - Making BR2_TARGET_ROOTFS_CPIO select BR2_PACKAGE_UTIL_LINUX_MOUNT?
>>
>> - Optionally adding that line if BR2_PACKAGE_UTIL_LINUX_MOUNT?
>
>  All of this is fairly complicated for something that is really a corner case.
>
>
>> - Adding a note to fs/cpio/Config.in that root filesystem will become
>> read-only only after corresponding line in /etc/fstab is processed
>> successfully?
>
>  The thing is, depending on various circumstances, the kernel may start the
> rootfs either rw or ro. It's pretty complicated. It's not even specific for
> cpio/initramfs (except that in that case it always starts rw so it's in fact
> simpler).
>
>  You *might* add a comment like that in the BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
> option, but even that is a bit too much detail I think.
>
>
>> I am more than happy to alter the patch to accommodate for either of
>> the above or just drop it and keep this change private to my buildroot
>> customization layer.
>
>  Yes, just put your version of /init in your fs-overlay and Bob's your uncle.
>

OK sounds good. Disregard this patch then.

Thanks,
Andrey Smirnov