New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
System V Compatbility Shutdown/Reboot Commands Should Honor Inhibitors and Send PrepareForShutdown DBus Signal When Run As Root #22058
Comments
|
The reason for the behavior is when using systemctl to execute or schedule the reboot, it ultimately lands in start_special() which calls logind_reboot(), while using legacy commands simply sets the arg_action and lets the system drop to release_busses() . Thus, there is no check of inhibitors nor any DBus PrepareForShutdown signal when using the legacy commands. While this looks to be by design, I don't believe utilizing the inhibitor capabilities of systemd should be limited to changing a decades long used set of commands to halt, reboot and poweroff the box. It there is a desire to maintain a clear line between the two systems reboot behaviors, perhaps a better approach would be to introduce a configuration flag - SystemVLegacyShutdownBehavior={yes,no} - which would allow the users to choose to the current behavior of /usr/sbin/{shtdown,reboot,halt,poweroff} - to either behave in a legacy manner or to follow the systemd standard which would include honoring inhibitors and sending a PrepareForShutdown . Would the configuration parameter fit better in logind.conf or system.conf ? |
|
Apparently not? systemd/src/systemctl/systemctl-compat-halt.c Line 147 in 04499a7
systemd/src/systemctl/systemctl-compat-halt.c Line 165 in 04499a7
|
|
halt_main() is only called when arg_action is ACTION_KEXEC . systemd/src/systemctl/systemctl.c Lines 1148 to 1154 in 3989bdc
|
|
Ack'd. Thank you, I actually was not aware of Fallthrough on switch statements. Using legacy commands, the inhibitors are not honored and no PrepareForShutdown DBus signal is emitted. Updating issue accordingly. I did indeed insert a debug statement in halt_main() and see it is getting reached. So the bug is elsewhere. |
|
systemd/src/systemctl/systemctl-compat-halt.c Lines 156 to 175 in 3989bdc
The issue seems to be stemming from calling legacy System V commands as root. Bypassing this check and calling logind_reboot() when results in the desired behavior - the shutdown delay inhibitor is honored and the PrepareForShutdown signal is emitted on the bus. Need to check start_special() and see what's going on in there that's different. However, I did throw a quick debug line in systemctl-start-special.c:213 and I can see that calling systemctl reboot as root does indeed reach it at: systemd/src/systemctl/systemctl-start-special.c Lines 201 to 213 in 3989bdc
I believe this check being altered to call logind_reboot regardless of being root or not should resolve the problem. |
|
It doesn't seem like inhibitor locks are supposed to have effect on root though: |
|
I think that functionality should be discussed further, though today - systemctl issued halt/reboot/poweroff commands are indeed honoring them as expected. So the behavior should be the same across the two interfaces, while I believe the behavior of systemctl executed actions is the right and desired behavior. The inhibitors can be forcefully ignored via parameters if that is desired, so root will always be able to bypass them and reboot the box. However, there may be points in time where the desire is there to prevent the reboot even as root. Particularly in the case of highly regulated environments subject to strict auditing, and in environments of 100k+ servers where the inhibitors could also be leveraged to more effectively deliver system updates. I've confirmed that block inhibitors are definitely ignored by root and have zero effect, which would make sense. The delay inhibitor has the protection of InhibitDelayMaxSec and thus I believe the current behavior of systemctl honoring them is the correct and more useful approach. |
|
So perhaps a more appropriate patch would be to make honoring inhibitors as root an optional configurable parameter, for both systemctl and the legacy system v commands? Or aim to just make system v commands behave the same as systemctl commands do currently? Personally, I feel the more appropriate path is to make system v legacy commands behave the same for now, and if there's desire to add a configurable ability to disable honoring inhibitors as root, then that should likely be another patch... ? |
|
I don't think so, the legacy commands are legacy, we don't change their behaviour or it will break backward compatibility. And adding a new option is also unnecessary, if you can update your script to use a new option with the legacy command, then you can also update it to use the supported command instead. |
|
It's not always that simple. Modifying decades worth of infra scripts, API's, user scripts, provisioning scripts and etc in a 100k+ server infrastructure that's government regulated, not to mention retrain every sys admin not to use "reboot" or "shutdown -r" - just wouldn't work. The code in place clearly shows that system v commands were intended to honor the inhibitors. Whether someone types "systemctl restart" or "reboot" should be irrelevant. If the box is running systemd, then the features and functionality provided by systemd should be available even if running the old "commands." Otherwise, it feels like you're saying "reboot" and "shutdown" are deprecated in the eyes of systemd, despite being standardized across every unix platform. At the very least, we should implement a configuration option - ie SystemVLegacyShutdownBehavior - and make this configurable. |
|
Additionally, what backward compatibility are you mentioning with regards to reboots? The biggest part of the system v compatibility is the init scripts and the handling of them as unit files. I'm unaware of what you mean by breaking backward compatibility by honoring inhibitors when using the legacy commands. On that note, it's already broken by the fact that scheduled reboots (/usr/sbin/shutdown -r +1) follow the same code logic as executing it via systemctl... |
|
You can't claim it both ways: either decades worth of infra scripts etc cannot be changed, and then adding a new option doesn't help since they won't be using it, or they can be changed to use a new option, which means they can also be changed to use the non-legacy commands if they want the new features. Changing the behaviour unconditionally for legacy commands is a non-starter, as it's a backward compat breakage. If 'reboot' suddenly doesn't reboot anymore, people will get mad. |
|
I believe there may be some confusion. Decades of infra scripts cannot be changed to go from calling /usr/sbin/reboot and /usr/sbin/shutdown -r now to calling systemctl restart and systemctl halt . Changing a configuration parameter in system.conf or login.conf is easy. Changing the behavior to honor inhibitors won't cause the box not to reboot. As I suggested, a new option in system.conf or login.conf can be introduced - SystemVLegacyShutdownBehavior - which by default would keep things exactly as they are. When flipped to no, inhibitors would be honored as desired. But again, it's already inconsistent in terms of behavior. Run the reproducer I have attached to the issue at the top. So likewise, it can't be partially backward compatible. It is either fully backward compatible or isn't at all. And in its current form, it's half and half. |
|
Patch in the works. I'm hoping that the concepts I'm implementing will give comfort in ensuring the legacy behavior remains, with the option to be enabled to change the behavior. Patch details:
I believe this will give the most flexibility by maintaining the current functionality but giving an option to leverage inhibitors with the current legacy commands, as well as providing a failsafe to force the reboot/halt even when its already inhibited if the feature is enabled. Note: This does not change the current behavior of blocking inhibitors which are ignored by root. This patch will aim solely to introduce new functionality when using legacy shutdown commands that must be explicitly enabled for those who wish to use it. The patch is functioning and written; I'm awaiting legal clearance from my organization to submit the patch for review. |
|
You may want to take a look at #21838 |
|
Thanks. My patch will need to be refactored to account for the changes you've made in halt_main() , so I'll take a look and start working on that. I'm hoping to have legal approval this upcoming week. I'm happy to help continue refactoring and cleaning up once I get through that hurdle. |
|
Inhibitors do not exist on Sysv, hence the SysV compat commands don't support them. Use the new commands if you want the new stuff. |
|
I appreciate you taking the time to chime in and engage the discussion. Hopefully I'll be able to convey my view on why this patch is a good direction. While I wish it were as simple as just using the new systemctl commands, it's unfortunately not always so. Our infrastructure is roughly 100k unix machines, with a little more than 80k employees total. We have decades worth of self-provisioning tools, update mechanisms and self-service utilities that can't all be re-written to use "systemctl {reboot, halt}" etc. We really {want,need} to leverage the inhibitors for auditing, regulatory and infrastructure maintenance purposes before shutdowns occur. We used to do this on sysv prior with a shutdown rc we customized. Systemd has changed a lot, in particular the system update portion. The inhibitors are a part of systemd. The system v compat commands exist to ease that transition and allow admins to still maintain and control the box in a manner they're familiar with without having to reinvent the wheel on how to reboot, halt or poweroff. Inhibitors are used with system v commands for non-root users, which system v didn't have support to do. While it makes sense not to allow a scenario where root could be unintentionally blocked from rebooting, it also makes sense to give the sys admins the option of leveraging the features provided by systemd regardless of the reboot or shutdown command they use. Additionally, scheduling a shutdown (/usr/sbin/shutdown -r +1) will honor inhibitors now as is, showing another inconsistency in the expectations. This has been a recurring topic which hasn't gained any traction; I'm just the first one that I can tell who has taken the time to write a patch that also takes in to consideration the view of everyone concerned about introducing a different behavior with system v compat commands. We're running systemd. If we execute the command as "reboot" or "systemctl reboot" - all we want is for the box to reboot. If we're using inhibitors, we want the inhibitors honored regardless of the command used to reboot the box. I believe the correct approach is to give the system administrators the ability to choose to leverage the enhanced functionality systemd provides them with regards to inhibitors and the PrepareForShutdown DBus signal. Our intended use-case is to monitor for PrepareForShutdown DBus signal and to trigger a series of events, including doing system updates to eliminate the dual-reboots introduced by the system-update target. Many of our boxes are 2-4 sockets, 1-3TB of ram, 1-8 network interfaces - so the reboots in a dual-reboot world can take almost an hour in some cases due to having to reboot twice with the system updates. Additionally, the inhibitors will allow us to trigger a set of actions that we will use for auditing and regulatory compliance measures. However, we can't modify all of the scripts across our entire infrastructure to leverage the systemctl commands because we in the core infrastructure team do not own most of them and have zero control over the commands being executed by some of those utilities. We'd really prefer not to wrap shutdown and reboot commands because this doesn't allow for an easy drop-in replacement for upgrades and the organization is a bit risk-averse with regards to wrapping such commands. |
|
couldn't you replace /sbin/shutdown with a shell wrapper calling |
|
No - I've had that discussion with stakeholders until we were all blue in the face. Shutdown wrappers are ugly and prohibited entirely, and that is true in most corporate environments with this size and type of regulation. That same discussion has been had in multiple closed issues of the same topic, i.e. one regarding "poweroff" and systemctl poweroff not behaving the same. The exact discussion regarding replacing the symlinks for sysv commands was had and even in his environment it was undesired. |
Did they provide any solid reasoning for this? |
|
Because it's ugly and presents more issues than it needs to with upgrades etc. Why would we want to wrap the shutdown and reboot commands if we could simply flip a switch in the login.conf ? When you're working in environments of this size, they rely on what's in the product and what the vendor supports - as unfortunate as that may sound. Custom wrappers for /sbin/{shutdown,reboot,halt} and etc aren't anything senior managers are willing to entertain. Additionally, why are we allowing non-root users to leverage inhibitors using sysv shutdown commands without even giving an option to allow root users to leverage them? |
|
The reason I got in touch with the shutdown logic was partially also motivated by the inhibitor inconsistency actually. My private use case is a rpi that does some regular sensor readings. The reading and processing may take a variable amount of seconds so makes sense to place an inhibitor to avoid rebooting in the middle. In that case it doesn't really matter whether eg some automatic OS update timer asks for reboot or someone logged in on the console. So IMHO it makes sense to enforce inhibitors on server side, no matter what the entry point is. As I've accidentally rebooted the machine at an inconvenient time as root I'd also argue that an interactive root should be denied unless asking for explicit override :-) |
|
Disregard, I just saw your pull request was merged. Your merge fixed my issue. Just one thing now: when using force, it should skip using logind_reboot and go straight to halting the system to prevent calling inhibitors. |
|
Issue was fixed in adefc87 |
Description of Behavior
System V compatibility shutdown/reboot/halt commands (ie via /usr/sbin/{shutdown,reboot}) do not exhibit the same behavior as a systemctl {reboot,halt} when run as root.
Most notable is that inhibitors appear to be skipped and a PrepareForShutdown DBus signal is not emitted when immediate action via /usr/sbin/{reboot,shutdown,halt,poweroff} are executed.
This behavior changes when using /usr/sbin/shutdown in a scheduled manner - i.e. /usr/sbin/shutdown -r +1 . In this case, delay inhibitors are honored and the DBus signal goes out, giving an inconsistent behavior in shutdown, reboot and poweroff operations between systemctl and the System V compatibility commands.
Example Test Case
Install required python depedencies:
Use the following sample script:
Run the above script.
As root, issue a systemctl reboot command. The inhibitor is honored, the PrepareForShutdown signal is sent and the inhibitor actions the signal.
Repeat the test, this time issuing a /usr/sbin/reboot or /usr/sbin/halt command. The inhibitor will not be triggered and the system will immediately go down.
Again repeat the test, running /usr/sbin/shutdown -r +1 . After one minute, the inhibitor will be honored, the PrepareForShutdown signal is sent and the inhibitor actions the signal.
Expected behavior:
Describe the solution you'd like
Legacy shutdown, halt and poweroff commands should be able to take advantage of the inhibitors, as well as the PepareForShutdown signal, even when run as root - as they are for systemctl reboot/halt commands.
Other References
This is similar to #15239
This looks to be due to logind_reboot() not being called in halt_main() if the reboot was executed by root.
systemd/src/systemctl/systemctl-compat-halt.c
Lines 156 to 175 in 3989bdc
Bypassing the check and calling logind_reboot() yields the expected behavior.
The systemd version you checked that didn't have the feature you are asking for
systemd 250 (250)
+PAM +AUDIT +SELINUX -APPARMOR +IMA +SMACK +SECCOMP +GCRYPT +GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN +IPTC +KMOD +LIBCRYPTSETUP +LIBFDISK +PCRE2 -PWQUALITY +P11KIT +QRENCODE +BZIP2 +LZ4 +XZ +ZLIB +ZSTD -BPF_FRAMEWORK +XKBCOMMON +UTMP +SYSVINIT default-hierarchy=unified
The text was updated successfully, but these errors were encountered: