Looks like the systemctl links are gone but not the pm-utils ones

Helmut Grohne helmut at subdivi.de
Mon Nov 20 15:05:31 GMT 2023


Hi,

I've spend the better part of today on this thanks to Freexian.

On Mon, Nov 20, 2023 at 12:35:58AM +0100, Helmut Grohne wrote:
> Different reproducer:
> 
> mmdebstrap trixie /dev/null http://deb.debian.org/debian --include=systemd-sysv,molly-guard --customize-hook='sed -i -e s/trixie/sid/ $1/etc/apt/sources.list' --chrooted-customize-hook='apt-get update' --customize-hook='test -e $1/lib/molly-guard/reboot' --chrooted-customize-hook='apt-get -y install systemd-sysv' --customize-hook='ls -l $1/lib/molly-guard/'

> I've dug into dpkg and usually when it moves a file from / to /usr,
> it'll first unpack the new file (unknowingly replacing the existing old
> one) and then removes the old one (via pkg_remove_old_files). During
> that removal, it has a check to see whether the file to be removed
> happens to match one of the files it just installed and skips the
> removal in that case. For some reason, this safety check does not work
> when the file is diverted.

I retried this a few times and still think it is correct. As a
consequence, the original approach of duplicating diversions cannot work
at all. As soon as we have two diversions whose targets equal up to
aliasing, we run into this problem and to make matters worse, we are
loosing a file that we just unpacked. We have no way of keeping (and
later restoring) it via any kind of maintainer scripts. Therefore we
really cannot do the duplicate diversion approach. It was a nice idea,
but doesn't work. Back to the drawing board.

We still must have two diversions to as unpacking either of the
locations from another package would clobber the location we want
diverted. What changes is that the diversion targets must differ in more
than aliasing.

I think an example would help. Say we divert /sbin/halt. We must divert
both /sbin/halt and /usr/sbin/halt or we risk clobbering our copy. If we
divert the latter to /usr/lib/molly-guard/halt, we must not divert the
former to /lib/molly-guard/halt or the file will be lost in unpack. So
the later might become /lib/molly-guard/halt.usr-is-merged or something.

This is fairly inconvenient as molly-guard doesn't know about
halt.usr-is-merged. I think there is three possible ways to deal with
this:
 a) molly-guard tries both locations.
 b) We add a dpkg-trigger on /sbin/halt such that when
    /usr/lib/molly-guard/halt is missing but
    /lib/molly-guard/halt.usr-is-merged is not, we can add a symlink.
 c) We give up on supporting /sbin/halt (as opposed to /usr/sbin/halt)
    and simply declare Breaks against any provider of /sbin/halt.

I argue that we should be selecting that last option, because /sbin/halt
is something that we want to go away entirely. When we release trixie,
we want all of them moved to /usr/sbin/halt. So we'd have versioned
Breaks for systemd-sysv, sysvinit-core, runit-init and finit-sysv. Note
that since Breaks does not prevent concurrent unpack, we must still
install the extra diversion. Note that since the other package must
declare Conflicts with molly-guard, molly-guard cannot use Conflicts
without impacting upgrades. The mutual conflict would require apt to
temporarily remove molly-guard (or /sbin/init!) and that is known to
impact upgrades.  Also note that since sysvinit-core and others have not
yet moved, molly-guard must now declare unversioned Breaks for all of
them for as long as they have not moved and once they moved, it can add
a version to the Breaks.

So I went ahead an implemented this. Since I didn't want to fall into my
trap of too little testing again, I wrote some test cases (attached) and
to my surprise found another problem!

/sbin/halt actually is a symbolic link to /bin/systemctl. In 255,
/usr/sbin/halt becomes a symlink to ../bin/systemctl. Observe how we
turned an absolute symlink into a relative once since we no longer
cross a toplevel directory in line with policy. As we move this relative
symlink to /usr/lib/molly-guard, it becomes a broken symbolic link. So
if you now install molly-guard and systemd-sysv in unstable, these links
are gone. You may then reinstall systemd-sysv and see them reinstated.
And now they're broken.

There is no easy way to fix this beyond moving these programs to a
different name in the same directory. I suggest
/usr/sbin/halt.no-molly-guard. Appending a suffix is what most
diversions do and this avoids breaking symlinks.

I've attached the prospective change to this mail. It passes piuparts
and it passes my own test cases. I definitely think it should be
reviewed before uploaded. One thing I already see for possible
improvement is that since we declare Breaks against all providers of
/sbin/halt, this diversion does not actually have to persist beyond
postinst. Once molly-guard is configured, we know that any broken
package is no longer installed nor unpacked. A molly-guard.postinst
could remove the diversion of /sbin/halt to
/sbin/halt.no-molly-guard.usr-is-merged. Then we are only left with one
diversion per command which also makes this a lot less confusing.

So rather than rushing this now, I hope to elicit some feedback and
review. I've made some non-trivial choices here and want you to
understand the trade-offs involved. Changing plans after upload would
complicate maintainer scripts to handle upgrades. I hope that systemd
people can provide some patience as we sort out their fallout. And if
this leaves more questions than answers, by all means ask. You wouldn't
be the first to raise an important matter via an innocuous looking
question.

If you wonder about the strange synax I introduced in maintainer
scripts, see
https://salsa.debian.org/jelmer/lintian-brush/-/merge_requests/39 and
feel free to remove my markup.

More fundamentally, I'll also have to rework the DEP17 section M18 as it
doesn't work the way it is currently pictured.

Thanks for bearing with me

Helmut
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testcase.sh
Type: application/x-sh
Size: 2986 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-systemd-maintainers/attachments/20231120/e627e01d/attachment.sh>
-------------- next part --------------
TESTS= \
	-_molly \
	molly_molly \
	sidmolly_molly \
	newmolly_rmmolly \
	systemd_molly \
	sysvinit_molly \
	systemd-newmolly_rmmolly \
	sysvinit-newmolly_rmmolly \
	molly-systemd_molly \
	molly-sysvinit_molly \
	sidmolly-systemd_molly \
	sidmolly-sysvinit_molly \
	newmolly_systemd \
	newmolly-systemd_systemd \
	newmolly-systemd_rmsystemd \
	newmolly-systemd_rmmolly \
	newmolly-systemd_sysvinit \
	newmolly_sidsystemd_rmsystemd \
	newmolly_sidsystemd_rmmolly \
	newmolly_sidsystemd_sysvinit \
	newmolly_sysvinit \
	newmolly-sysvinit_rmsysvinit \
	newmolly-sysvinit_rmmolly \
	newmolly-sysvinit_systemd \

all: $(foreach t,$(TESTS),testout/$(t))

testout/%:
	./testcase.sh "$(firstword $(subst _, ,$*))" "$(lastword $(subst _, ,$*))" >"$@" 2>&1; echo $$? >> "$@"
-------------- next part --------------
diff -Nru molly-guard-0.8.1/debian/changelog molly-guard-0.8.1+nmu1/debian/changelog
--- molly-guard-0.8.1/debian/changelog	2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/debian/changelog	2023-11-20 09:18:25.000000000 +0100
@@ -1,3 +1,10 @@
+molly-guard (0.8.1+nmu1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Attempt to fix the /usr-merge fallout.
+
+ -- Helmut Grohne <helmut at subdivi.de>  Mon, 20 Nov 2023 09:18:25 +0100
+
 molly-guard (0.8.1) unstable; urgency=medium
 
   * Upload to unstable
diff -Nru molly-guard-0.8.1/debian/control molly-guard-0.8.1+nmu1/debian/control
--- molly-guard-0.8.1/debian/control	2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/debian/control	2023-11-20 09:18:25.000000000 +0100
@@ -23,6 +23,7 @@
           systemd,
           sysvinit,
           upstart
+Breaks: systemd-sysv (<< 255), sysvinit-core, finit-sysv, runit-init
 Description: protects machines from accidental shutdowns/reboots
  The package installs a shell script that overrides the existing
  shutdown/reboot/halt/poweroff/coldreboot/pm-hibernate/pm-suspend* commands
diff -Nru molly-guard-0.8.1/debian/molly-guard.postrm molly-guard-0.8.1+nmu1/debian/molly-guard.postrm
--- molly-guard-0.8.1/debian/molly-guard.postrm	2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/debian/molly-guard.postrm	2023-11-20 09:18:25.000000000 +0100
@@ -20,11 +20,8 @@
 case "$1" in
     remove)
         for cmd in halt poweroff reboot shutdown coldreboot ; do
-            dpkg-divert --package molly-guard --no-rename --remove /sbin/$cmd
-            dpkg-divert --package molly-guard --no-rename --remove "/usr/sbin/$cmd"
-	    if test -e "/usr/lib/molly-guard/$cmd"; then
-                mv "/usr/lib/molly-guard/$cmd" "/usr/sbin/$cmd"
-            fi
+            dpkg-divert --package molly-guard --rename --remove /sbin/$cmd
+            dpkg-divert --package molly-guard --rename --remove "/usr/sbin/$cmd"
         done
 
         for cmd in pm-hibernate pm-suspend pm-suspend-hybrid ; do
diff -Nru molly-guard-0.8.1/debian/molly-guard.preinst molly-guard-0.8.1+nmu1/debian/molly-guard.preinst
--- molly-guard-0.8.1/debian/molly-guard.preinst	2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/debian/molly-guard.preinst	2023-11-20 09:18:25.000000000 +0100
@@ -14,35 +14,55 @@
 
 case "$1" in
     install|upgrade)
-        mkdir -p /usr/lib/molly-guard
-
         # Cleanup erroneous diversions added in 0.6.0
         for cmd in pm-hibernate pm-suspend pm-suspend-hybrid ; do
             dpkg-divert --package molly-guard --rename --remove /sbin/$cmd
         done
 
         for cmd in halt poweroff reboot shutdown coldreboot ; do
-            dpkg-divert --package molly-guard --divert "/usr/lib/molly-guard/$cmd" --no-rename --add "/usr/sbin/$cmd"
-            # DEP17 M18 duplicated diversion. Can be removed after trixie.
-            dpkg-divert --package molly-guard --divert "/lib/molly-guard/$cmd" --no-rename --add "/sbin/$cmd"
-	    # Avoid --rename as long as we need duplicated diversions.
+            # begin-remove-after: trixie
+            # Remove possible pre-/usr-merge diversion
+            dpkg-divert --package molly-guard --divert "/lib/molly-guard/$cmd" --no-rename --remove  "/sbin/$cmd"
+            if test "$(dpkg-divert --truename "/usr/sbin/$cmd")" = "/usr/lib/molly-guard/$cmd"; then
+                dpkg-divert --package molly-guard --divert "/usr/lib/molly-guard/$cmd" --no-rename --remove "/usr/sbin/$cmd"
+                if test -e "/usr/lib/molly-guard/$cmd"; then
+                    mv "/usr/lib/molly-guard/$cmd" "/usr/sbin/$cmd.no-molly-guard"
+                fi
+                if test -e "/lib/molly-guard/$cmd"; then
+                    mv "/lib/molly-guard/$cmd" "/usr/sbin/$cmd.no-molly-guard.usr-is-merged"
+                fi
+            fi
+            # end-remove-after: trixie
+            # DEP17 M18 duplicated diversion. Can be --removed after trixie.
+            dpkg-divert --package molly-guard --divert "/sbin/$cmd.no-molly-guard.usr-is-merged" --no-rename --add "/sbin/$cmd"
+            # Add post-/usr-merge diversion meant to stay.
+            dpkg-divert --package molly-guard --divert "/usr/sbin/$cmd.no-molly-guard" --no-rename --add "/usr/sbin/$cmd"
+            # Avoid --rename as long as we need duplicated diversions.
             if test "$1" = install; then
                 if test -e "/usr/sbin/$cmd"; then
-                    mv "/usr/sbin/$cmd" "/usr/lib/molly-guard/$cmd"
+                    mv "/usr/sbin/$cmd" "/usr/sbin/$cmd.no-molly-guard"
                 fi
                 if test -e "/sbin/$cmd"; then
-                    mv "/sbin/$cmd" "/usr/lib/molly-guard/$cmd"
+                    mv "/sbin/$cmd" "/sbin/$cmd.no-molly-guard.usr-is-merged"
                 fi
             fi
         done
 
         for cmd in pm-hibernate pm-suspend pm-suspend-hybrid ; do
-            if test "$(dpkg-divert --truename "/usr/sbin/$cmd")" = "/lib/molly-guard/$cmd"; then
-                dpkg-divert --package molly-guard --divert "/lib/molly-guard/$cmd" --no-rename --remove "/usr/sbin/$cmd"
-                dpkg-divert --package molly-guard --divert "/usr/lib/molly-guard/$cmd" --no-rename --add "/usr/sbin/$cmd"
+            # begin-remove-after: trixie
+            truename="$(dpkg-divert --truename "/usr/sbin/$cmd")"
+            if test "${truename#/usr}" = "/lib/molly-guard/$cmd"; then
+                dpkg-divert --package molly-guard --divert "$truename" --no-rename --remove "/usr/sbin/$cmd"
+                dpkg-divert --package molly-guard --divert "/usr/sbin/$cmd.no-molly-guard" --no-rename --add "/usr/sbin/$cmd"
+                if test -e "/usr/lib/molly-guard/$cmd" -o -h "/usr/lib/molly-guard/$cmd"; then
+                     mv "/usr/lib/molly-guard/$cmd" "/usr/sbin/$cmd.no-molly-guard"
+                fi
             else
-                dpkg-divert --package molly-guard --divert "/usr/lib/molly-guard/$cmd" --rename "/usr/sbin/$cmd"
+            # end-remove-after: trixie
+                dpkg-divert --package molly-guard --divert "/usr/sbin/$cmd.no-molly-guard" --rename "/usr/sbin/$cmd"
+            # begin-remove-after: trixie
             fi
+            # end-remove-after: trixie
         done
     ;;
 
diff -Nru molly-guard-0.8.1/shutdown.in molly-guard-0.8.1+nmu1/shutdown.in
--- molly-guard-0.8.1/shutdown.in	2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/shutdown.in	2023-11-20 09:18:25.000000000 +0100
@@ -13,11 +13,10 @@
 SCRIPTSDIR="@cfgdir@/run.d"
 
 CMD="${0##*/}"
-EXEC="@REALPATH@/$CMD"
 
 case "$CMD" in
   halt|reboot|shutdown|poweroff|coldreboot|pm-hibernate|pm-suspend|pm-suspend-hybrid)
-    if [ ! -f $EXEC ]; then
+    if ! EXEC=$(command -v "$CMD.no-molly-guard"); then
       echo "E: not a regular file: $EXEC" >&2
       exit 4
     fi


More information about the Pkg-systemd-maintainers mailing list