Merge f2f9c199d1 into e0258ac886

The change was supposed to be about respecting inhibitors, but
it was extended to also error out when there are active user
sessions, which was not intentional. Previously systemctl skipped
all checks if the caller was root or root-equivalent. Restore the
previous behaviour and again avoid blocking systemctl reboot by root
if there are active sessions, as long as there are no active
inhibitors.

Fixes https://github.com/systemd/systemd/issues/34086

Follow-up for 804874d26a

NEWS

@@ -35,17 +35,17 @@ CHANGES WITH 257 in spe:
           Accept=yes sockets too, where it was previously silently ignored and
           "connection" was used unconditionally.
 
-        * systemd-logind now always obeys inhibitor locks, where previously it
-          ignored locks taken by the caller or when the caller was root. A
+        * systemd-logind now always obeys block inhibitor locks, where previously
+          it ignored locks taken by the caller or when the caller was root. A
           privileged caller can always close the other sessions, remove the
           inhibitor locks, or use --force or --check-inhibitors=no to ignore the
           inhibitors. This change thus doesn't affect security, since everything
           that was possible before at a given privilege level is still possible,
           but it should make the inhibitor logic easier to use and understand,
-          and also help avoiding accidental reboots and shutdowns. New 'delay-weak'
-          and 'block-weak' inhibitor modes were added, if taken they will make
-          the inhibitor lock work as in the previous versions. Inhibitor locks
-          can also be taken by remote users (subject to polkit policy).
+          and also help avoiding accidental reboots and shutdowns. New 'block-weak'
+          inhibitor modes were added, if taken they will make the inhibitor lock
+          work as in the previous versions. Inhibitor locks can also be taken by
+          remote users (subject to polkit policy).
 
         * systemd-nspawn will now mount the unified cgroup hierarchy into a
           container if no systemd installation is found in a container's root

docs/INHIBITOR_LOCKS.md

@@ -42,8 +42,8 @@ If such a lock is taken the operation will fail (but still may be overridden if
 The InhibitDelayMaxSec= setting in [logind.conf(5)](http://www.freedesktop.org/software/systemd/man/logind.conf.html) controls the timeout for this. This is intended to be used by applications which need a synchronous way to execute actions before system suspend but shall not be allowed to block suspend indefinitely.
 This mode is only available for _sleep_ and _shutdown_ locks.
 
-3. _block-weak_ and _delay-weak_ that work as the non-weak counterparts, but that in addition may be ignored
-automatically and silently under certain circumstances, unlike the formers which are always respected.
+3. _block-weak_ that works as its non-weak counterpart, but that in addition may be ignored
+automatically and silently under certain circumstances, unlike the former which is always respected.
 
 Inhibitor locks are taken via the Inhibit() D-Bus call on the logind Manager object:
 

man/org.freedesktop.login1.xml

@@ -672,16 +672,18 @@ node /org/freedesktop/login1 {
 #define SD_LOGIND_SKIP_INHIBITORS                (UINT64_C(1) << 4)
       </programlisting>
       <para>When the <varname>flags</varname> is 0 then these methods behave just like the versions without
-      flags. Since systemd version 256 <constant>SD_LOGIND_ROOT_CHECK_INHIBITORS</constant> (0x01) is deprecated,
-      and active inhibitors are always honoured by default for privileged users too, and a new flag
-      <constant>SD_LOGIND_SKIP_INHIBITORS</constant> (0x04) can be specified to bypass inhibitors. When
-      <constant>SD_LOGIND_KEXEC_REBOOT</constant> (0x02) is set, then <function>RebootWithFlags()</function>
-      performs a kexec reboot if kexec kernel is loaded. When <constant>SD_LOGIND_SOFT_REBOOT</constant>
-      (0x04) is set, or <constant>SD_LOGIND_SOFT_REBOOT_IF_NEXTROOT_SET_UP</constant> (0x08) is set and a
-      new root file system has been set up on <literal>/run/nextroot/</literal>, then
-      <function>RebootWithFlags()</function> performs a userspace reboot only.
-      <constant>SD_LOGIND_SOFT_REBOOT_IF_NEXTROOT_SET_UP</constant> and
-      <constant>SD_LOGIND_KEXEC_REBOOT</constant> can be combined, with soft-reboot having precedence.</para>
+      flags. Since systemd version 257 active inhibitors are honoured by default for privileged users too.
+      <constant>SD_LOGIND_ROOT_CHECK_INHIBITORS</constant> (0x01) now only applies to weak inhibitors, to
+      request that they honoured for privileged users too, since they ignore them by default. A new flag
+      <constant>SD_LOGIND_SKIP_INHIBITORS</constant> (0x04) can be specified to bypass all types of
+      inhibitors. When <constant>SD_LOGIND_KEXEC_REBOOT</constant> (0x02) is set, then
+      <function>RebootWithFlags()</function> performs a kexec reboot if kexec kernel is loaded. When
+      <constant>SD_LOGIND_SOFT_REBOOT</constant> (0x04) is set, or
+      <constant>SD_LOGIND_SOFT_REBOOT_IF_NEXTROOT_SET_UP</constant> (0x08) is set and a new root file system
+      has been set up on <literal>/run/nextroot/</literal>, then <function>RebootWithFlags()</function>
+      performs a userspace reboot only. <constant>SD_LOGIND_SOFT_REBOOT_IF_NEXTROOT_SET_UP</constant> and
+      <constant>SD_LOGIND_KEXEC_REBOOT</constant> can be combined, with soft-reboot having precedence.
+      </para>
 
       <para><function>SetRebootParameter()</function> sets a parameter for a subsequent reboot operation.
       See the description of <command>reboot</command> in
@@ -738,11 +740,10 @@ node /org/freedesktop/login1 {
       should be a short human readable string identifying the reason why the lock is taken. Finally,
       <varname>mode</varname> is either <literal>block</literal> or <literal>delay</literal> which encodes
       whether the inhibit shall be consider mandatory or whether it should just delay the operation to a
-      certain maximum time, while the <literal>block-weak</literal> and <literal>delay-weak</literal>
-      variants will create an inhibitor that is automatically ignored in some circumstances. The method
-      returns a file descriptor. The lock is released the moment this file descriptor and all its duplicates
-      are closed. For more information on the inhibition logic see
-      <ulink url="https://systemd.io/INHIBITOR_LOCKS">Inhibitor Locks</ulink>.
+      certain maximum time, while the <literal>block-weak</literal> and variants will create an inhibitor
+      that is automatically ignored in some circumstances. The method returns a file descriptor. The lock is
+      released the moment this file descriptor and all its duplicates are closed. For more information on
+      the inhibition logic see <ulink url="https://systemd.io/INHIBITOR_LOCKS">Inhibitor Locks</ulink>.
       </para>
     </refsect2>
 

man/systemd-inhibit.xml

@@ -92,13 +92,12 @@
       <varlistentry>
         <term><option>--mode=</option></term>
 
-        <listitem><para>Takes <literal>block</literal>, <literal>delay</literal>,
-        <literal>block-weak</literal> or <literal>delay-weak</literal> and describes how the lock is
-        applied. If <literal>block</literal> is used (the default), the lock prohibits any of the requested
-        operations without time limit, and only privileged users may override it. If
-        <literal>delay</literal> is used, the lock can only delay the requested operations for a limited
-        time. If the time elapses, the lock is ignored and the operation executed. The time limit may be
-        specified in
+        <listitem><para>Takes <literal>block</literal>, <literal>delay</literal>, or
+        <literal>block-weak</literal> and describes how the lock is applied. If <literal>block</literal> is
+        used (the default), the lock prohibits any of the requested operations without time limit, and only
+        privileged users may override it. If <literal>delay</literal> is used, the lock can only delay the
+        requested operations for a limited time. If the time elapses, the lock is ignored and the operation
+        executed. The time limit may be specified in
         <citerefentry><refentrytitle>logind.conf</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
         Note that <literal>delay</literal> is only available for <literal>sleep</literal> and
         <literal>shutdown</literal>. In addition, the weak variants will automatically and silently be

shell-completion/zsh/_systemd-inhibit

@@ -33,6 +33,6 @@ _arguments \
     '--what=[Operations to inhibit]:options:_systemd-inhibit_what' \
     '--who=[A descriptive string who is inhibiting]:who is inhibiting:' \
     '--why=[A descriptive string why is being inhibited]:reason for the lock:' \
-    '--mode=[One of block, block-weak, delay, or delay-weak]:lock mode:( block block-weak delay delay-weak )' \
+    '--mode=[One of block, block-weak, or delay]:lock mode:( block block-weak delay )' \
     '--list[List active inhibitors]' \
     '*:commands:_systemd-inhibit_commands'

src/login/inhibit.c

@@ -157,7 +157,7 @@ static int help(void) {
                "                          handle-lid-switch\n"
                "     --who=STRING         A descriptive string who is inhibiting\n"
                "     --why=STRING         A descriptive string why is being inhibited\n"
-               "     --mode=MODE          One of block, block-weak, delay, or delay-weak\n"
+               "     --mode=MODE          One of block, block-weak, or delay\n"
                "     --list               List active inhibitors\n"
                "\nSee the %s for details.\n",
                program_invocation_short_name,

src/login/logind-dbus.c

@@ -3622,7 +3622,7 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error
                                          "Invalid mode specification %s", mode);
 
         /* Delay is only supported for shutdown/sleep */
-        if (IN_SET(mm, INHIBIT_DELAY, INHIBIT_DELAY_WEAK) && (w & ~(INHIBIT_SHUTDOWN|INHIBIT_SLEEP)))
+        if (mm == INHIBIT_DELAY && (w & ~(INHIBIT_SHUTDOWN|INHIBIT_SLEEP)))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
                                          "Delay inhibitors only supported for shutdown and sleep");
 

src/login/logind-inhibit.c

@@ -371,7 +371,7 @@ InhibitWhat manager_inhibit_what(Manager *m, bool block) {
 
         HASHMAP_FOREACH(i, m->inhibitors)
                 if (i->started &&
-                    ((!block && IN_SET(i->mode, INHIBIT_DELAY, INHIBIT_DELAY_WEAK)) ||
+                    ((!block && i->mode == INHIBIT_DELAY) ||
                      (block && IN_SET(i->mode, INHIBIT_BLOCK, INHIBIT_BLOCK_WEAK))))
                         what |= i->what;
 
@@ -424,13 +424,13 @@ bool manager_is_inhibited(
                         continue;
 
                 if ((block && !IN_SET(i->mode, INHIBIT_BLOCK, INHIBIT_BLOCK_WEAK)) ||
-                    (!block && !IN_SET(i->mode, INHIBIT_DELAY, INHIBIT_DELAY_WEAK)))
+                    (!block && i->mode != INHIBIT_DELAY))
                         continue;
 
                 if (ignore_inactive && pidref_is_active_session(m, &i->pid) <= 0)
                         continue;
 
-                if (IN_SET(i->mode, INHIBIT_BLOCK_WEAK, INHIBIT_DELAY_WEAK) && ignore_uid && i->uid == uid)
+                if (i->mode == INHIBIT_BLOCK_WEAK && ignore_uid && i->uid == uid)
                         continue;
 
                 if (!inhibited ||
@@ -531,7 +531,6 @@ static const char* const inhibit_mode_table[_INHIBIT_MODE_MAX] = {
         [INHIBIT_BLOCK]      = "block",
         [INHIBIT_BLOCK_WEAK] = "block-weak",
         [INHIBIT_DELAY]      = "delay",
-        [INHIBIT_DELAY_WEAK] = "delay-weak"
 };
 
 DEFINE_STRING_TABLE_LOOKUP(inhibit_mode, InhibitMode);

src/login/logind-inhibit.h

@@ -22,7 +22,6 @@ typedef enum InhibitMode {
         INHIBIT_BLOCK,
         INHIBIT_BLOCK_WEAK,
         INHIBIT_DELAY,
-        INHIBIT_DELAY_WEAK,
         _INHIBIT_MODE_MAX,
         _INHIBIT_MODE_INVALID = -EINVAL,
 } InhibitMode;

src/systemctl/systemctl-logind.c

@@ -204,6 +204,10 @@ int logind_check_inhibitors(enum action a) {
         if (r < 0)
                 return bus_log_parse_error(r);
 
+        /* root respects inhibitors since v257 but keeps ignoring sessions by default */
+        if (arg_check_inhibitors < 0 && c == 0 && geteuid() == 0)
+                return 0;
+
         /* Check for current sessions */
         sd_get_sessions(&sessions);
         STRV_FOREACH(s, sessions) {