Skip to content

Conversation

@navi-desu
Copy link
Member

if for whatever reason, /run/openrc exists in the rootfs before first boot, we'd still end up with an invalid fd. while users are not really supposed to have anything in /run (under the tmpfs mount), should they do, it's better to not fail

if for whatever reason, /run/openrc exists in the rootfs before first
boot, we'd still end up with an invalid fd. while users are not really
supposed to have anything in /run (under the tmpfs mount), should they
do, it's better to not fail

clearing the dirfds as we create svcdir also means we don't need a new
librc api function to clear it, for now at least.
@navi-desu navi-desu merged commit 22bfd3a into master Jun 29, 2025
10 checks passed
@navi-desu navi-desu deleted the more-dirfd-fixes branch June 29, 2025 00:28
@martinetd
Copy link
Contributor

FWIW I still reproduce boot failures with a read-only rootfs that contains /run/openrc, rc_parallel=YES, and tmpfs mounted over /run/openrc a bit later:

   OpenRC 0.62.5 is starting up Linux 5.10.238-0-at (aarch64) [PODMAN]

 * Mounting /proc ... [ ok ]
 * Mounting /run ... [ ok ]
 * /run/openrc: creating directory
 * /run/lock: creating directory
 * /run/lock: correcting owner
fopen '/run/openrc/deptree': Read-only file system
fopen '/run/openrc/depconfig': Read-only file system

 * Failed to update the dependency tree
 [ !! ]
 * failed to load deptree
 * Caching service dependencies ... * Caching service dependencies ... [ ok ]
 * Checking log filesystems ... * Preparing overlayfs over / ... * Mounting /sys ... * Remounting devtmpfs on /dev ... [ ok ]
^ services started in incorrect order because dependency tree is wrong

So apparently it somehow opened /run/openrc before mounting /run, then mounted /run, created /run/openrc and still used the old read-only fd for /run/openrc? I don't think there are multiple processes at this point with rc_parallel so I'm not sure why the fd wasn't cleared properly...

but anyway I think it's fine to require /run to be empty as long as it's known, so I didn't check further on this; please let me know if you'd like me to dig a bit into this as it's probably easier for me to reproduce this.

@navi-desu
Copy link
Member Author

navi-desu commented Jul 7, 2025

but anyway I think it's fine to require /run to be empty as long as it's known, so I didn't check further on this; please let me know if you'd like me to dig a bit into this as it's probably easier for me to reproduce this.

while it'd be fine, it'd be somewhat breaking behaviour compared to 0.61, so i'd like to fix it

if you could either attempt to debug it, or send me a full strace -o openrc -ff of the boot, or even just the podman image (idk if that works like that?), that'd help a ton!

@martinetd
Copy link
Contributor

while it'd be fine, it'd be somewhat breaking behaviour compared to 0.61, so i'd like to fix it

Fair enough!

if you could either attempt to debug it, or send me a full strace -o openrc -ff of the boot, or even just the podman image (idk if that works like that?), that'd help a ton!

So looking at strace output, the problem with this patch seems to be that /usr/libexec/rc/sh/init.sh (sh/init.sh.Linux.in) creates the directory with checkpath -d "$RC_SVCDIR", so the
_rc_deptree_load() -> rc_deptree_update_needed() -> mkdir() that comes just after (run_program(INITSH); in do_sysinit()) doesn't create the directory, and the fd cache isn't cleared.

Unfortunately we can't remove that checkpath as init.sh also copies a few things in /run/openrc (and e.g. writes to softlevel file), so I'd be tempted to take the big hammer and just always clear dirfd in do_sysinit after running INITSH, as that is what mounts /run ?

even just the podman image (idk if that works like that?)

I'm running this on a real board; but could reproduce with a container (the ugly first command is just the only way I know to build a container as a one-liner... it should work with docker instead of podman as well if you prefer/already have docker)

printf '%s\n' 'FROM docker.io/alpine:3.22' 'RUN apk add --no-cache openrc strace' 'RUN mkdir /run/openrc' | podman build -t openrc-test -f -  /var/empty/
podman run -ti --rm --privileged openrc-test sh -c 'mount -o remount,ro / && /sbin/openrc sysinit'

You can run sh instead of the non-interactive command and run strace openrc sysinit or whatever.

@navi-desu
Copy link
Member Author

so I'd be tempted to take the big hammer and just always clear dirfd in do_sysinit after running INITSH, as that is what mounts /run ?

that works, but, i didn't want to add clear_dirfds to librc's public api (yet at least) -- i think i can just clear the dirfds unconditionally for now

navi-desu added a commit that referenced this pull request Jul 7, 2025
init.sh creates the directory, which means we never actually do it here
early at boot

Fixes: 9c3f974
Fixes: #893
navi-desu added a commit that referenced this pull request Jul 8, 2025
init.sh creates the directory, which means we never actually do it here
early at boot

Fixes: 22bfd3a
Fixes: #893
navi-desu added a commit that referenced this pull request Jul 8, 2025
init.sh creates the directory, which means we never actually do it here
early at boot

Fixes: 22bfd3a
Fixes: #893
navi-desu added a commit that referenced this pull request Jul 22, 2025
init.sh creates the directory, which means we never actually do it here
early at boot

Fixes: 22bfd3a
Fixes: #893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants