Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
Control: tags -1 + patch On Fri, Dec 01, 2023 at 05:50:29PM +0100, Helmut Grohne wrote: > I am very sorry to break this promise. It's complicated. Almost two weeks later, I'm back with what I hope is a solution. > So there will be no patch for now, because there still are unsolved > problems. While looking into Emilio's problem and fixing the version > constraints, I figured that test cases are failing. In particular, there > are situations where dpkg unpacks systemd-sysv from unstable at a time > where molly-guard from bookworm is still unpacked. This causes files > from systemd-sysv to be irrevocably lost. I fear we're going back to the > drawing board. If you want details, try #1057199, but I caution that > it's not for the faint of heart and I already have apt and dpkg > developers assist, which is awesome. This has also resulted in systemd-sysv bug #1057220. I've considered the options forwards and backwards. Since this loss scenario does not involve a trixie molly-guard at all, it needs a solution in systemd only. Discussions have resulted in two key insights: * If working with apt (rather than dpkg directly), this scenario can only happen if molly-guard and systemd-sysv have a mutual conflict (or breaks). We should avoid this, see below. * We cannot really prevent this from happening otherwise as the unpack of the lost file and the loss happen in the same unpack phase. At the time of this writing, my preferred solution is restoring the lost files in postinst. Fortunately, they are all symlinks in the case of systemd-sysv, so restoring them is a rather simple matter. And this is what the attached systemd patch does. > Among the ideas circulated by multiple DDs, the one that seems most > promising to me was from Julian Andres Klode. He suggests that we add a > "barrier" package "usrmerge-support". It would have versioned Conflicts > with molly-guard and systemd-sysv would declare Pre-Depends on > usrmerge-support. This approach can be used to emulate the semantics > that I hoped to get from Conflicts, because Pre-Depends imply that at > the time usrmerge-support.postinst runs, molly-guard must be removed or > upgraded and systemd-sysv is not yet unpacked. Totally untested though. > > I fear there is not much you can do at this point. This is not the option I'm going for now. Rather, given that systemd can paper over the loss we can make the loss very unlikely by having molly-guard not declare Breaks against systemd-sysv. As a result, apt no longer sees a mutual conflict and no longer schedules temporary removal. Thus, the loss scenario (usually) does not happen (though systemd-sysv still mitigates it). Not having Breaks comes at a cost though. Now the trixie molly-guard must work with the bookworm systemd-sysv that still places the diverted files in /sbin rather than /sbin. Remember that we earlier learned that the diversion targets must differ in more than aliasing, so molly-guard must now deal with locating diverted tools in two distinct locations, which kinda is ugly, but practically works. This also implies that the duplicated diversions are not as temporary as we wished. We cannot just delete the aliased ones in postinst. We have to keep them throughout the trixie release. Without loss of generality, we'll divert tools like this: /usr/sbin/halt -> /usr/sbin/halt.no-molly-guard /sbin/halt -> /sbin/halt.no-molly-guard.usr-is-merged The latter of these can be removed after trixie and the former is permanent. Emilio noticed that we must call the diverted tools by their true name. While systemd uses strstr and this happens to work, sysvinit-core uses strcmp, so we need to provide the exact argv[0]. Unfortunately "exec -a" is as bash feature not supported by POSIX shells such as dash. I've refrained from changing the shell interpreter from sh to bash and instead invoke bash for the purpose of performing that "exec -a". Changing it to become a bash script would also work. Testing. I've significantly improved my earlier tests to understand many of the situations. I've also significantly changed the test scenarios (given that we also have to test a changed systemd-sysv now). All of these pass now. None of the tests invoke the systemd mitigation (as expected since molly-guard no longer Breaks). I've run molly-guard through piuparts and installed it into a VM and manually performed these tests: * local poweroff * poweroff via ssh asks for hostname * poweroff --help # verified output * poweroff --invalidarg # proper error message * poweroff '--invalid --arg' # no accidental word splitting Finally, I ran the reproducer from #1057220 and verified that the mitigation in systemd-sysv actually is invoked and works. So now I am attaching the result of my work. I invite people to review it (even though I understand that this is a complex matter). In particular, I am also interested in what kind of tests I should be performing in addition. I also encourage you to upload these
Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
Hi Francois, On Mon, Nov 27, 2023 at 04:05:55PM -0800, Francois Marier wrote: > On 2023-11-27 at 03:54:16, Helmut Grohne (hel...@subdivi.de) wrote: > > I don't have time to update the patch right now. Let me promise an update > > this week, ok? I am very sorry to break this promise. It's complicated. > My apologies for not responding earlier, but this is a rather thorny problem > to solve and I have not had the mental "bandwidth" to dig into this yet. > > I wanted however to express my sincere appreciation for all of the work you > have put into both understanding this problem and coming up with a solution. Well, in essence I am driving the completion of the /usr-merge transition. You may argue that I have caused all this trouble even if that is an oversimplification. The way you support solving this matter is appreciated. I fully understand that fixing this requires significant effort that you cannot just provide spontaneously. So there will be no patch for now, because there still are unsolved problems. While looking into Emilio's problem and fixing the version constraints, I figured that test cases are failing. In particular, there are situations where dpkg unpacks systemd-sysv from unstable at a time where molly-guard from bookworm is still unpacked. This causes files from systemd-sysv to be irrevocably lost. I fear we're going back to the drawing board. If you want details, try #1057199, but I caution that it's not for the faint of heart and I already have apt and dpkg developers assist, which is awesome. Among the ideas circulated by multiple DDs, the one that seems most promising to me was from Julian Andres Klode. He suggests that we add a "barrier" package "usrmerge-support". It would have versioned Conflicts with molly-guard and systemd-sysv would declare Pre-Depends on usrmerge-support. This approach can be used to emulate the semantics that I hoped to get from Conflicts, because Pre-Depends imply that at the time usrmerge-support.postinst runs, molly-guard must be removed or upgraded and systemd-sysv is not yet unpacked. Totally untested though. I fear there is not much you can do at this point. Let me also clarify that I am not really looking into this for molly-guard's sake. For one thing, molly-guard provides an instance of a problem class repeated multiple times in the archive. I hope that once we get molly-guard right, the other instances become easier. For another, your responsiveness and care is appreciated. Stay tuned Helmut
Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
On 2023-11-27 at 03:54:16, Helmut Grohne (hel...@subdivi.de) wrote: > I don't have time to update the patch right now. Let me promise an update > this week, ok? Hi Helmut, My apologies for not responding earlier, but this is a rather thorny problem to solve and I have not had the mental "bandwidth" to dig into this yet. I wanted however to express my sincere appreciation for all of the work you have put into both understanding this problem and coming up with a solution. Francois
Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
Hi, On Sat, Nov 25, 2023 at 12:50:26AM +0100, Helmut Grohne wrote: > Hope this is good. Would you mind uploading it to experimental? I sat down with Emilio and had him review (big thanks!) my patch and while he thinks that my problem analysis and diversion management will now work, he figured another problem. A major change is that we now rename the diverted files in the same directory rather than keeping the name in a different directory to avoid breaking symbolic links. Unfortunately, systemctl uses the program name to figure out what it should be doing and it has no clue what "halt.no-molly-guard" is supposed to mean. So when we run the original program, we need to now run it with exec -a $cmd $cmd.no-molly-guard. I don't have time to update the patch right now. Let me promise an update this week, ok? Helmut
Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
Hi, On Mon, Nov 20, 2023 at 04:05:31PM +0100, Helmut Grohne wrote: > 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. I have implemented the removal of unnecessary diversions in postinst and attach an updated patch. It still passes all of the tests from my earlier mail and also piuparts. > More fundamentally, I'll also have to rework the DEP17 section M18 as it > doesn't work the way it is currently pictured. I updated it. Hope this is good. Would you mind uploading it to experimental? Helmut 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.0 +0100 +++ molly-guard-0.8.1+nmu1/debian/changelog 2023-11-20 09:18:25.0 +0100 @@ -1,3 +1,10 @@ +molly-guard (0.8.1+nmu1) UNRELEASED; urgency=medium + + * Non-maintainer upload. + * Attempt to fix the /usr-merge fallout. (Closes: #1056279) + + -- Helmut Grohne 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/control2023-11-11 23:02:55.0 +0100 +++ molly-guard-0.8.1+nmu1/debian/control 2023-11-20 09:18:25.0 +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.postinst molly-guard-0.8.1+nmu1/debian/molly-guard.postinst --- molly-guard-0.8.1/debian/molly-guard.postinst 1970-01-01 01:00:00.0 +0100 +++ molly-guard-0.8.1+nmu1/debian/molly-guard.postinst 2023-11-20 09:18:25.0 +0100 @@ -0,0 +1,14 @@ +#!/bin/sh +set -e + +# begin-remove-after: trixie +if test "$1" = configure; then +for cmd in halt poweroff reboot shutdown coldreboot; do +dpkg-divert --package molly-guard --no-rename --remove "/sbin/$cmd" +done +fi +# end-remove-after + +#DEBHELPER# + +exit 0 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.0 +0100 +++ molly-guard-0.8.1+nmu1/debian/molly-guard.postrm2023-11-20 09:18:25.0 +0100 @@ -16,18 +16,19 @@ # for details, see http://www.debian.org/doc/debian-policy/ or # the debian-policy package - +# begin-remove-after: trixie 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 +abort-install|abort-upgrade|failed-upgrade) +for cmd in halt poweroff shutdown coldreboot; do +dpkg-divert --package molly-guard --rename --remove "/sbin/$cmd" done +;; +esac +# end-remove-after -for cmd in pm-hibernate pm-suspend pm-suspend-hybrid ; do +case "$1" in +remove) +for cmd in halt poweroff reboot shutdown coldreboot pm-hibernate pm-suspend pm-suspend-hybrid ; do dpkg-divert --package molly-guard --rename --remove /usr/sbin/$cmd done 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.preinst2023-11-11 23:02:55.0 +0100 +++ molly-guard-0.8.1+nmu1/debian/molly-guard.preinst 2023-11-20 09:18:25.0 +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
Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
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
Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
Control: tags -1 + confirmed Control: clone -1 -2 Control: reassign -2 systemd-sysv Control: found -2 255-rc2-1 Control: retitle -2 duplicated diversions are still broken by moved files On Sun, Nov 19, 2023 at 02:08:35PM -0800, Francois Marier wrote: > I'm also a little confused by the diverts. Perhaps something changed in > systemd (which owns the ultimate underlying symlinks)? I was sure I had this properly tested and yet it doesn't work as expected. I'm sorry for having gotten this wrong. Reproducer: mmdebstrap bookworm /dev/null http://deb.debian.org/debian --include=systemd-sysv,molly-guard --customize-hook='sed -i -e s/bookworm/sid/ $1/etc/apt/sources.list' --chrooted-customize-hook='apt-get update' --chrooted-customize-hook='apt-get -y install systemd-sysv' --customize-hook='ls -l $1/lib/molly-guard/' 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/' Specifically, the files vanish on upgrading systemd-sysv such that /sbin/reboot moves to /usr/sbin/reboot. I should have seen this failure in earlier tests. 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. So I have a vague understanding of what is wrong here, but no solution yet. For the time being, I duplicate this into a blocker bug for systemd-sysv to prevent it from migrating to testing until we figure out a solution. Sorry for the inconvenience. Helmut
Bug#1056279: Looks like the systemctl links are gone but not the pm-utils ones
CCing Helmut who wrote the initial patch for systemd 255+ support (see Bug#1055510). I also see the same thing: $ ls -lh /usr/lib/molly-guard/ Permissions Size User Group Date Modified Name .rwxr-xr-x 3,4k root root 11 nov 14:02 molly-guard* lrwxrwxrwx31 root root 14 nov 2019 pm-hibernate -> /usr/lib/pm-utils/bin/pm-action* lrwxrwxrwx31 root root 14 nov 2019 pm-suspend -> /usr/lib/pm-utils/bin/pm-action* lrwxrwxrwx31 root root 14 nov 2019 pm-suspend-hybrid -> /usr/lib/pm-utils/bin/pm-action* $ sudo reboot --help E: not a regular file: /usr/lib/molly-guard/reboot I'm also a little confused by the diverts. Perhaps something changed in systemd (which owns the ultimate underlying symlinks)? Francois -- https://fmarier.org/
