[PATCH] multipath.rules.in: fix race in unit cancel-multipath-wait-$kernel

[Antonio Alvarez Feijoo <antonio.feijoo@suse.com>]

The transient unit cancel-multipath-wait-$kernel creates a timer and
service with "Conflicts=initrd-cleanup.service":

- sda: /usr/lib/udev/rules.d/56-multipath.rules ...
    --on-active=1.000000 ...
- sda: ... 'Running timer as unit: cancel-multipath-wait-sda.timer'
- sda: ... 'Will run service as unit:
    cancel-multipath-wait-sda.service'

This introduces a race not always reproducible: there is a 1 second
window where this transient unit can start after
initrd-cleanup.service/start is enqueued but is still held back. When
that happens, starting cancel-multipath-wait-$kernel.service pulls in
a stop job for initrd-cleanup.service and the queued start job is
cancelled, the initrd is left with an empty job queue and hangs
forever, before switch-root:

- initrd-parse-etc.service: Triggering OnSuccess= dependencies.
- initrd-cleanup.service: Installed new job
    initrd-cleanup.service/start as 115
- initrd-cleanup.service: starting held back, waiting for:
    dracut-pre-pivot.service
- initrd-cleanup.service: Job 115 initrd-cleanup.service/start
    finished, result=canceled
- initrd-cleanup.service: Installed new job
    initrd-cleanup.service/stop as 133
- initrd-cleanup.service: Job 133 initrd-cleanup.service/stop
    finished, result=done

"Conflicts=" is symmetric: if a unit has a "Conflicts=" requirement
on a set of other units, then starting it will stop all of them and
starting any of them will stop it. This is safe against
shutdown.target, whose jobs are enqueued irreversibly: a unit
starting during shutdown fails to start rather than cancelling the
shutdown. It is not safe against initrd-cleanup.service, whose start
job is an ordinary job enqueued by initrd-parse-etc.service via
"OnSuccess=".

Ordering "Before=initrd-cleanup.service" is enough and is not
destructive: a trigger already in flight completes before cleanup
runs, and a unit with no job does not delay it. Tearing the
transient units down before switch-root is already handled by the
isolate to initrd-switch-root.target, which stops everything that
is not "IgnoreOnIsolate=yes".

Signed-off-by: Antonio Alvarez Feijoo <antonio.feijoo@suse.com>
---
 multipath/multipath.rules.in | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/multipath/multipath.rules.in b/multipath/multipath.rules.in
index 2ac1972f..7b605dd4 100644
--- a/multipath/multipath.rules.in
+++ b/multipath/multipath.rules.in
@@ -68,8 +68,18 @@ ENV{.SAVED_FM_WAIT_UNTIL}=="?*", GOTO="pretend_mpath"
 # the --on-active timeout.
 #
 # We must trigger an "add" event because LVM2 will only act on those.
-
-RUN+="@SYSDIR_BIN@/systemd-run --unit=cancel-multipath-wait-$kernel --description 'cancel waiting for multipath siblings of $kernel' --no-block --timer-property DefaultDependencies=no --timer-property Conflicts=shutdown.target --timer-property Before=shutdown.target --timer-property Conflicts=initrd-cleanup.service --timer-property Before=initrd-cleanup.service --timer-property AccuracySec=500ms --property DefaultDependencies=no --property Conflicts=shutdown.target --property Before=shutdown.target --property Conflicts=initrd-cleanup.service --property Before=initrd-cleanup.service --on-active=$env{FIND_MULTIPATHS_WAIT_UNTIL} @SYSDIR_BIN@/udevadm trigger --action=add $sys$devpath"
+#
+# These units are ordered Before=initrd-cleanup.service so that a trigger that
+# is already in flight completes before the initrd switches root. They must
+# *not* use Conflicts=initrd-cleanup.service: unlike shutdown.target, whose
+# jobs are enqueued irreversibly, initrd-cleanup.service/start is a plain job
+# enqueued by initrd-parse-etc.service via OnSuccess=. Starting a unit that
+# conflicts with it therefore cancels that job instead of failing to start,
+# and OnSuccess= is never retried, so the initrd would hang forever with an
+# empty job queue. Stopping these units before switch-root is already
+# guaranteed by the isolate to initrd-switch-root.target.
+
+RUN+="@SYSDIR_BIN@/systemd-run --unit=cancel-multipath-wait-$kernel --description 'cancel waiting for multipath siblings of $kernel' --no-block --timer-property DefaultDependencies=no --timer-property Conflicts=shutdown.target --timer-property Before=shutdown.target --timer-property Before=initrd-cleanup.service --timer-property AccuracySec=500ms --property DefaultDependencies=no --property Conflicts=shutdown.target --property Before=shutdown.target --property Before=initrd-cleanup.service --on-active=$env{FIND_MULTIPATHS_WAIT_UNTIL} @SYSDIR_BIN@/udevadm trigger --action=add $sys$devpath"
 
 LABEL="pretend_mpath"
 ENV{DM_MULTIPATH_DEVICE_PATH}="1"
-- 
2.51.0