Re: [make-initrd] [PATCH 2/2] runtime: Fix console lock shadowing by /dev mount

[Leonid Krivoshein <klark.devel@gmail.com>]

Привет!


On 10/16/26 3:06 PM, Alexey Gladkov wrote:
> /etc/rc.d/rc acquires an exclusive lock on /dev/console before starting
> runlevel services. When the mountfs service mounts devtmpfs over /dev,
> the original /dev/console inode on rootfs is shadowed by a new inode on
> devtmpfs. Consequently, rc holds the lock on the shadowed inode while
> processes started after mountfs lock the new /dev/console inode without
> waiting for rc to finish starting services. In particular, with the
> rdshell boot parameter, rootdelayd acquires the console lock and starts
> the emergency shell in the middle of runlevel 3 instead of after all
> services have started.
> 
> Once the console lock is preserved across mountfs, the network-up init
> service deadlocks against its parent /etc/rc.d/rc because network-up
> requests a shared lock on the console while rc already holds an
> exclusive lock for the duration of the runlevel scripts.
> 
> Use a dedicated /.initrd/console.lock file in /.initrd so the lock
> remains on the same inode across /dev mount operations, and drop the
> redundant lock acquisition from the synchronous network-up init script.

Хорошо, что нашлось решение и есть в сообществе люди, умеющие не только 
репортить баги, но и разбирающиеся в них, предлагающие патчи! Я всего-то 
хотел уточнить, поскольку вносил изменения в свою документацию, а тут 
столько всего поменялось. :-)

Но есть вопросы к этим изменениям, см. ниже...


> 
> Fixes: 598a4b241a4f ("Rewrite console locking")
> Reported-by: Leonid Krivoshein <klark.devel@gmail.com>
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
>   features/fsck/data/lib/uevent/handlers/040-mountdev         | 2 +-
>   features/luks/data/lib/uevent/handlers/085-luks             | 2 +-
>   features/network/data/etc/rc.d/init.d/network-up            | 3 ---
>   features/runtime/data/etc/rc.d/rc                           | 2 +-
>   features/runtime/data/init                                  | 1 +
>   .../runtime/data/lib/uevent/handlers/mount/900-localdev     | 2 +-
>   features/runtime/data/sbin/rootdelayd                       | 6 +++---
>   features/sshfsroot/data/lib/uevent/handlers/040-sshfsroot   | 2 +-
>   features/zfs/data/sbin/ueventd-zfs                          | 2 +-
>   9 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/features/fsck/data/lib/uevent/handlers/040-mountdev b/features/fsck/data/lib/uevent/handlers/040-mountdev
> index 53208f0d..9e9f06ea 100755
> --- a/features/fsck/data/lib/uevent/handlers/040-mountdev
> +++ b/features/fsck/data/lib/uevent/handlers/040-mountdev
> @@ -56,7 +56,7 @@ handler()
>   }
>   
>   exec 0</dev/console >/dev/console 2>&1
> -fd_lock 90 /dev/console
> +fd_lock 90 /.initrd/console.lock
Получается, здесь сначала выполняется открытие файла консоли, только 
потом её захват. Не лучше ли наоборот или это непринципиально?

Коль блокировка консоли используется внутри make-initrd во многих 
местах, нет ли смысла скрыть её внутреннюю реализацию как раз на случай 
таких изменений? Что-то типа fd_lock_console() / fd_unlock_console().

Насколько безопасно использовать статически заданный дескриптор для этой 
блокировки? Не лучше ли его определять динамически?


>   
>   for e in "$eventdir"/mountdev.*; do
>   	[ -e "$e" ] || break
> diff --git a/features/luks/data/lib/uevent/handlers/085-luks b/features/luks/data/lib/uevent/handlers/085-luks
> index db5a217b..423082ad 100755
> --- a/features/luks/data/lib/uevent/handlers/085-luks
> +++ b/features/luks/data/lib/uevent/handlers/085-luks
> @@ -366,7 +366,7 @@ handler() {
>   }
>   
>   exec 0</dev/console >/dev/console 2>&1
> -fd_lock 90 /dev/console
> +fd_lock 90 /.initrd/console.lock
>   
>   rc=0
>   for e in "$eventdir"/luks.*; do
> diff --git a/features/network/data/etc/rc.d/init.d/network-up b/features/network/data/etc/rc.d/init.d/network-up
> index 9ec3b978..ae1532e0 100755
> --- a/features/network/data/etc/rc.d/init.d/network-up
> +++ b/features/network/data/etc/rc.d/init.d/network-up
> @@ -13,14 +13,11 @@
>   
>   msg="Network up"
>   
> -. shell-locks
>   . network-sh-functions
>   
>   # wait for network modules
>   udevadm settle --timeout=3
>   
> -fd_rlock 90 /dev/console
> -
>   ifaces=' '
>   while :; do
>   	set -- /sys/class/net/*
> diff --git a/features/runtime/data/etc/rc.d/rc b/features/runtime/data/etc/rc.d/rc
> index 02f85ae5..916fa390 100755
> --- a/features/runtime/data/etc/rc.d/rc
> +++ b/features/runtime/data/etc/rc.d/rc
> @@ -22,7 +22,7 @@ rcd="/etc/rc.d/rc$runlevel.d"
>   	exit 0
>   
>   exec </dev/console 2>&1
> -fd_lock 90 /dev/console
> +fd_lock 90 /.initrd/console.lock
>   
>   msg "INIT: Entering runlevel: $runlevel"
>   
> diff --git a/features/runtime/data/init b/features/runtime/data/init
> index 6fd887a4..f4c789a1 100755
> --- a/features/runtime/data/init
> +++ b/features/runtime/data/init
> @@ -11,6 +11,7 @@ export PATH=/sbin:/usr/sbin:/usr/local/sbin:/bin:/usr/bin:/usr/local/bin
>   
>   # Create storage for init environment
>   :> /.initrd/initenv
> +:> /.initrd/console.lock
>   
>   # Check and create the necessary device files
>   [ -d /dev ] ||
> diff --git a/features/runtime/data/lib/uevent/handlers/mount/900-localdev b/features/runtime/data/lib/uevent/handlers/mount/900-localdev
> index ddd33df2..ecc37b36 100755
> --- a/features/runtime/data/lib/uevent/handlers/mount/900-localdev
> +++ b/features/runtime/data/lib/uevent/handlers/mount/900-localdev
> @@ -78,7 +78,7 @@ while [ "$method" = localdev ]; do
>   	all_mounted && sysinit_found ||
>   		break
>   
> -	fd_lock 90 /dev/console
> +	fd_lock 90 /.initrd/console.lock
>   
>   	message "$QUEUE: session=$SESSION: $method ready. acting!"
>   
> diff --git a/features/runtime/data/sbin/rootdelayd b/features/runtime/data/sbin/rootdelayd
> index d61b2ae4..cad6efd6 100755
> --- a/features/runtime/data/sbin/rootdelayd
> +++ b/features/runtime/data/sbin/rootdelayd
> @@ -36,7 +36,7 @@ while :; do
>   	if [ -n "${RDSHELL-}" ]; then
>   		mkdir -p -- "$disabled"
>   
> -		fd_lock 90 /dev/console
> +		fd_lock 90 /.initrd/console.lock
>   		rdshell "Emergency shell" 90<&-
>   		fd_unlock 90
>   
> @@ -76,7 +76,7 @@ while :; do
>   
>   		msg="${msg:-The waiting time expired!}"
>   
> -		fd_lock 90 /dev/console
> +		fd_lock 90 /.initrd/console.lock
>   
>   		# Disallow console access
>   		if [ "${PANIC-}" = "0" ]; then
> @@ -101,7 +101,7 @@ while :; do
>   
>   	if [ -z "${QUIET-}" ] && [ -z "$first_iter" ] && [ $(( $delay % 15 )) -eq 0 ]; then
>   
> -		fd_lock 90 /dev/console
> +		fd_lock 90 /.initrd/console.lock
>   
>   		resume_checked &&
>   			msg="Waiting for root ${ROOT:+($ROOT) }..." ||
> diff --git a/features/sshfsroot/data/lib/uevent/handlers/040-sshfsroot b/features/sshfsroot/data/lib/uevent/handlers/040-sshfsroot
> index 6730a203..dfd140cf 100755
> --- a/features/sshfsroot/data/lib/uevent/handlers/040-sshfsroot
> +++ b/features/sshfsroot/data/lib/uevent/handlers/040-sshfsroot
> @@ -41,7 +41,7 @@ handler() {
>   }
>   
>   exec 0</dev/console >/dev/console 2>&1
> -fd_lock 90 /dev/console
> +fd_lock 90 /.initrd/console.lock
>   
>   rc=0
>   for e in "$eventdir"/sshfsroot.*; do
> diff --git a/features/zfs/data/sbin/ueventd-zfs b/features/zfs/data/sbin/ueventd-zfs
> index 8229eaee..e164d779 100755
> --- a/features/zfs/data/sbin/ueventd-zfs
> +++ b/features/zfs/data/sbin/ueventd-zfs
> @@ -104,7 +104,7 @@ while :; do
>   	fi
>   
>   	if resume_checked; then
> -		fd_lock 90 /dev/console
> +		fd_lock 90 /.initrd/console.lock
>   
>   		findmnt -sno TARGET -t zfs | sort -d -k1,1 |
>   		while read -r mnt; do

-- 
WBR, Leonid Krivoshein.