Package: plymouth
Version: 0.9.4-1.1
Severity: grave
Justification: may result in data loss

Hi,

I've noticed that when using offline-updates[1], remounting the root
filesystem read-only at shutdown fails with 'Device or resource busy'
(see the attached screenshot). As the result, the filesystem is left in
an unclean state, which may lead to data loss. Given that the installed
by default GNOME desktop environment currently provides no upgrade
method other than offline-updates, the impact is pretty wide.

It turned out that it is plymouth's fault. plymouthd runs early during
boot, typically from initrd. It runs with --mode=boot --attach-to-
session, so when the root filesystem is remounted read-write, it opens
/var/log/boot.log for writing and logs console messages there. When the
system is booting normally, this plymouthd instance is stopped either
by plymouth-quit.service in multi-user.target or by gdm.service which
stops plymouthd on its own. However, when performing offline-updates
the system doesn't boot any further than sysinit.target, so plymouthd
keeps running and logging to /var/log/boot.log. After all updates are
installed, a reboot is initiated, during which the running services are
stopped and remaining processes are killed. However, plymouthd uses a
trick to survive systemd's killing spree (see [2] or src/main.c:2240),
so it doesn't get killed, and by keeping /var/log/boot.log opened for
writing prevents remounting the root filesystem read-only.

I'm not sure how to best fix this issue. As a workaround, I added

ExecStartPre=-/bin/plymouth --wait quit

to plymouth-{halt,kexec,poweroff,reboot}.service-s (which run during
shutdown) to ensure that if plymouthd instance in --mode=boot is still
running, it is stopped before executing plymouthd in --mode=shutdown
(which doesn't do logging and therefor doesn't interfere with the
shutdown process). Aside from a small flicker during offline-updates
caused by reexecuting plymouthd, there should be no side effects. A
proposed patch is attached.

Another possible fix is to make plymouthd close the log file when its
mode changes to 'shutdown' by pk-offline-update. It makes sense and
seems trivial; however, it relies on the update service to switch
plymouth to that state, which doesn't look like a reliable solution in
the long term.

plymouthd can also be changed so that instead of just giving up if it
detects an already running instance, it will transfer its state
requested by command line options to that instance, so when, say,
plymouth-reboot.service runs plymouthd with --mode=shutdown, the
running instance actually switches to that mode and closes the log
file. Such change however, while looking like the most correct
solution, is likely not trivial, and is better be done by the upstream.

 [1] 
https://www.freedesktop.org/software/systemd/man/systemd.offline-updates.html
 [2] 
https://sources.debian.org/src/plymouth/0.9.4-1.1/src/main.c/#L2240
diff --git a/systemd-units/plymouth-halt.service.in b/systemd-units/plymouth-halt.service.in
index 38ae98c..ad7bafc 100644
--- a/systemd-units/plymouth-halt.service.in
+++ b/systemd-units/plymouth-halt.service.in
@@ -7,6 +7,8 @@ ConditionKernelCommandLine=!plymouth.enable=0
 ConditionVirtualization=!container
 
 [Service]
+# Terminate an already running plymouthd instance if there is one
+ExecStartPre=-@PLYMOUTH_CLIENT_DIR@/plymouth --wait quit
 ExecStart=@PLYMOUTH_DAEMON_DIR@/plymouthd --mode=shutdown --attach-to-session
 ExecStartPost=-@PLYMOUTH_CLIENT_DIR@/plymouth show-splash
 Type=forking
diff --git a/systemd-units/plymouth-kexec.service.in b/systemd-units/plymouth-kexec.service.in
index bed5eb7..df7b328 100644
--- a/systemd-units/plymouth-kexec.service.in
+++ b/systemd-units/plymouth-kexec.service.in
@@ -7,6 +7,8 @@ ConditionKernelCommandLine=!plymouth.enable=0
 ConditionVirtualization=!container
 
 [Service]
+# Terminate an already running plymouthd instance if there is one
+ExecStartPre=-@PLYMOUTH_CLIENT_DIR@/plymouth --wait quit
 ExecStart=@PLYMOUTH_DAEMON_DIR@/plymouthd --mode=shutdown --attach-to-session
 ExecStartPost=-@PLYMOUTH_CLIENT_DIR@/plymouth show-splash
 Type=forking
diff --git a/systemd-units/plymouth-poweroff.service.in b/systemd-units/plymouth-poweroff.service.in
index 7891e97..d4ebf28 100644
--- a/systemd-units/plymouth-poweroff.service.in
+++ b/systemd-units/plymouth-poweroff.service.in
@@ -7,6 +7,8 @@ ConditionKernelCommandLine=!plymouth.enable=0
 ConditionVirtualization=!container
 
 [Service]
+# Terminate an already running plymouthd instance if there is one
+ExecStartPre=-@PLYMOUTH_CLIENT_DIR@/plymouth --wait quit
 ExecStart=@PLYMOUTH_DAEMON_DIR@/plymouthd --mode=shutdown --attach-to-session
 ExecStartPost=-@PLYMOUTH_CLIENT_DIR@/plymouth show-splash
 Type=forking
diff --git a/systemd-units/plymouth-reboot.service.in b/systemd-units/plymouth-reboot.service.in
index 1d57789..f665740 100644
--- a/systemd-units/plymouth-reboot.service.in
+++ b/systemd-units/plymouth-reboot.service.in
@@ -7,6 +7,8 @@ ConditionKernelCommandLine=!plymouth.enable=0
 ConditionVirtualization=!container
 
 [Service]
+# Terminate an already running plymouthd instance if there is one
+ExecStartPre=-@PLYMOUTH_CLIENT_DIR@/plymouth --wait quit
 ExecStart=@PLYMOUTH_DAEMON_DIR@/plymouthd --mode=shutdown --attach-to-session
 ExecStartPost=-@PLYMOUTH_CLIENT_DIR@/plymouth show-splash
 Type=forking

Reply via email to