install: allow adding plain templates to .wants/ or .requires/

Fixes #19437.

As reported in the bug:

> # drkonqi-coredump-processor@.service
>  ...
> [Install]
> WantedBy=systemd-coredump@.service
>
> The plan here is to have a systemd-coredump@ instance start the same %i for
> drkonqi-coredump-processor@. Works perfectly when creating the symlink manually
> ln -sv /usr/lib/systemd/system/drkonqi-coredump-processor@.service
> /etc/systemd/system/systemd-coredump@.service.wants/.

When DefaultInstance is set, we replace template references with
template@default-inst. But in this case we want to create a symlink for the
template name, so that systemd will fill in the instance from the
wanting/requiring unit. This is only possible for those units that actually
have an instance set, so we create the symlink only from .requires/ or .wants
of an instantiated unit (then this specific instance will be used), or a
template (than some instance will be inherited later).

Specifically:
...
[Install]
WantedBy=other@.service, fixed.service
DefaultInstance=inst

→ enable foo@.service creates other@.service.wants/foo@inst.service, and
other@a.service will want foo@inst.service, and other@b.service will want foo@inst.service,
and fixed.service will want foo@inst.service.

Without DefaultInstance,
→ enable foo@.service creates other@.service.wants/foo@.service, and
other@a.service would want foo@a.service, and other@b.service would want foo@b.service,
but enablement fails because no dependency can be created for fixed.service:

  Failed to enable unit, unit fixed.service is a non-template unit.

man/udevadm.xml

@@ -332,7 +332,7 @@
         </varlistentry>
         <varlistentry>
           <term><option>-g</option></term>
-          <term><option>--tag-match=<replaceable>PROPERTY</replaceable></option></term>
+          <term><option>--tag-match=<replaceable>TAG</replaceable></option></term>
           <listitem>
             <para>Trigger events for devices with a matching tag. When this option is specified multiple times,
             then each matching result is ANDed, that is, devices which have all specified tags are triggered.</para>

src/core/socket-bind.c

@@ -61,45 +61,47 @@ static int prepare_socket_bind_bpf(
                 deny_count++;
 
         if (allow_count > SOCKET_BIND_MAX_RULES)
-                return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL),
-                                "Maximum number of socket bind rules=%u is exceeded", SOCKET_BIND_MAX_RULES);
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_WARNING, SYNTHETIC_ERRNO(EINVAL),
+                                           "Maximum number of socket bind rules=%u is exceeded", SOCKET_BIND_MAX_RULES);
 
         if (deny_count > SOCKET_BIND_MAX_RULES)
-                return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL),
-                                "Maximum number of socket bind rules=%u is exceeded", SOCKET_BIND_MAX_RULES);
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_WARNING, SYNTHETIC_ERRNO(EINVAL),
+                                           "Maximum number of socket bind rules=%u is exceeded", SOCKET_BIND_MAX_RULES);
 
         obj = socket_bind_bpf__open();
         if (!obj)
-                return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOMEM), "Failed to open BPF object");
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_DEBUG, SYNTHETIC_ERRNO(ENOMEM),
+                                           "Failed to open BPF object");
 
         if (sym_bpf_map__resize(obj->maps.sd_bind_allow, MAX(allow_count, 1u)) != 0)
-                return log_unit_error_errno(u, errno,
-                                "Failed to resize BPF map '%s': %m", sym_bpf_map__name(obj->maps.sd_bind_allow));
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_WARNING, errno,
+                                           "Failed to resize BPF map '%s': %m", sym_bpf_map__name(obj->maps.sd_bind_allow));
 
         if (sym_bpf_map__resize(obj->maps.sd_bind_deny, MAX(deny_count, 1u)) != 0)
-                return log_unit_error_errno(u, errno,
-                                "Failed to resize BPF map '%s': %m", sym_bpf_map__name(obj->maps.sd_bind_deny));
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_WARNING, errno,
+                                           "Failed to resize BPF map '%s': %m", sym_bpf_map__name(obj->maps.sd_bind_deny));
 
         if (socket_bind_bpf__load(obj) != 0)
-                return log_unit_error_errno(u, errno, "Failed to load BPF object");
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_DEBUG, errno,
+                                           "Failed to load BPF object: %m");
 
         allow_map_fd = sym_bpf_map__fd(obj->maps.sd_bind_allow);
         assert(allow_map_fd >= 0);
 
         r = update_rules_map(allow_map_fd, allow);
         if (r < 0)
-                return log_unit_error_errno(
-                                u, r, "Failed to put socket bind allow rules into BPF map '%s'",
-                                sym_bpf_map__name(obj->maps.sd_bind_allow));
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_WARNING, r,
+                                           "Failed to put socket bind allow rules into BPF map '%s'",
+                                           sym_bpf_map__name(obj->maps.sd_bind_allow));
 
         deny_map_fd = sym_bpf_map__fd(obj->maps.sd_bind_deny);
         assert(deny_map_fd >= 0);
 
         r = update_rules_map(deny_map_fd, deny);
         if (r < 0)
-                return log_unit_error_errno(
-                                u, r, "Failed to put socket bind deny rules into BPF map '%s'",
-                                sym_bpf_map__name(obj->maps.sd_bind_deny));
+                return log_unit_full_errno(u, u ? LOG_ERR : LOG_WARNING, r,
+                                           "Failed to put socket bind deny rules into BPF map '%s'",
+                                           sym_bpf_map__name(obj->maps.sd_bind_deny));
 
         *ret_obj = TAKE_PTR(obj);
         return 0;

src/core/unit.c

@@ -1282,13 +1282,18 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) {
         }
 
         if (c->private_tmp) {
-                const char *p;
 
-                FOREACH_STRING(p, "/tmp", "/var/tmp") {
-                        r = unit_require_mounts_for(u, p, UNIT_DEPENDENCY_FILE);
-                        if (r < 0)
-                                return r;
-                }
+                /* FIXME: for now we make a special case for /tmp and add a weak dependency on
+                 * tmp.mount so /tmp being masked is supported. However there's no reason to treat
+                 * /tmp specifically and masking other mount units should be handled more
+                 * gracefully too, see PR#16894. */
+                r = unit_add_two_dependencies_by_name(u, UNIT_AFTER, UNIT_WANTS, "tmp.mount", true, UNIT_DEPENDENCY_FILE);
+                if (r < 0)
+                        return r;
+
+                r = unit_require_mounts_for(u, "/var/tmp", UNIT_DEPENDENCY_FILE);
+                if (r < 0)
+                        return r;
 
                 r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_TMPFILES_SETUP_SERVICE, true, UNIT_DEPENDENCY_FILE);
                 if (r < 0)

src/libsystemd/sd-device/device-internal.h

@@ -108,5 +108,6 @@ int device_set_devname(sd_device *device, const char *devname);
 int device_set_devtype(sd_device *device, const char *devtype);
 int device_set_devnum(sd_device *device, const char *major, const char *minor);
 int device_set_subsystem(sd_device *device, const char *_subsystem);
+int device_set_drivers_subsystem(sd_device *device);
 int device_set_driver(sd_device *device, const char *_driver);
 int device_set_usec_initialized(sd_device *device, usec_t when);

src/libsystemd/sd-device/device-private.c

@@ -382,12 +382,20 @@ void device_seal(sd_device *device) {
 }
 
 static int device_verify(sd_device *device) {
+        int r;
+
         assert(device);
 
         if (!device->devpath || !device->subsystem || device->action < 0 || device->seqnum == 0)
                 return log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL),
                                               "sd-device: Device created from strv or nulstr lacks devpath, subsystem, action or seqnum.");
 
+        if (streq(device->subsystem, "drivers")) {
+                r = device_set_drivers_subsystem(device);
+                if (r < 0)
+                        return r;
+        }
+
         device->sealed = true;
 
         return 0;
@@ -743,7 +751,7 @@ int device_rename(sd_device *device, const char *name) {
 
 int device_shallow_clone(sd_device *old_device, sd_device **new_device) {
         _cleanup_(sd_device_unrefp) sd_device *ret = NULL;
-        const char *subsystem;
+        const char *val;
         int r;
 
         assert(old_device);
@@ -757,17 +765,43 @@ int device_shallow_clone(sd_device *old_device, sd_device **new_device) {
         if (r < 0)
                 return r;
 
-        if (sd_device_get_subsystem(old_device, &subsystem) >= 0) {
-                r = device_set_subsystem(ret, subsystem);
+        if (sd_device_get_subsystem(old_device, &val) >= 0) {
+                r = device_set_subsystem(ret, val);
                 if (r < 0)
                         return r;
+
+                if (streq(val, "drivers")) {
+                        ret->driver_subsystem = strdup(old_device->driver_subsystem);
+                        if (!ret->driver_subsystem)
+                                return -ENOMEM;
+                }
         } else
                 ret->subsystem_set = true;
 
-        ret->devnum = old_device->devnum;
+        /* The device may be already removed. Let's copy minimal set of information to make
+         * device_get_device_id() work without uevent file. */
+
+        if (sd_device_get_property_value(old_device, "IFINDEX", &val) >= 0) {
+                r = device_set_ifindex(ret, val);
+                if (r < 0)
+                        return r;
+        }
+
+        if (sd_device_get_property_value(old_device, "MAJOR", &val) >= 0) {
+                const char *minor = NULL;
+
+                (void) sd_device_get_property_value(old_device, "MINOR", &minor);
+                r = device_set_devnum(ret, val, minor);
+                if (r < 0)
+                        return r;
+        }
+
+        /* And then read uevent file, but ignore errors, as some devices seem to return a spurious
+         * error on read, e.g. -ENODEV, and even if ifindex or devnum is set in the above,
+         * sd_device_get_ifindex() or sd_device_get_devnum() fails. See. #19788. */
+        (void) device_read_uevent_file(ret);
 
         *new_device = TAKE_PTR(ret);
-
         return 0;
 }
 

src/libsystemd/sd-device/sd-device.c

@@ -539,22 +539,17 @@ int device_read_uevent_file(sd_device *device) {
         if (r < 0)
                 return r;
 
+        device->uevent_loaded = true;
+
         path = strjoina(syspath, "/uevent");
 
         r = read_full_virtual_file(path, &uevent, &uevent_len);
-        if (r == -EACCES) {
-                /* empty uevent files may be write-only */
-                device->uevent_loaded = true;
-                return 0;
-        }
-        if (r == -ENOENT)
-                /* some devices may not have uevent files, see device_set_syspath() */
+        if (IN_SET(r, -EACCES, -ENOENT))
+                /* The uevent files may be write-only, or the device may not have uevent file. */
                 return 0;
         if (r < 0)
                 return log_device_debug_errno(device, r, "sd-device: Failed to read uevent file '%s': %m", path);
 
-        device->uevent_loaded = true;
-
         for (size_t i = 0; i < uevent_len; i++)
                 switch (state) {
                 case PRE_KEY:
@@ -779,7 +774,7 @@ int device_set_subsystem(sd_device *device, const char *_subsystem) {
         return free_and_replace(device->subsystem, subsystem);
 }
 
-static int device_set_drivers_subsystem(sd_device *device) {
+int device_set_drivers_subsystem(sd_device *device) {
         _cleanup_free_ char *subsystem = NULL;
         const char *syspath, *drivers, *p;
         int r;

src/shared/condition.c

@@ -675,6 +675,11 @@ static int condition_test_needs_update(Condition *c, char **env) {
         if (r > 0)
                 return b;
 
+        if (in_initrd()) {
+                log_debug("We are in an initrd, not doing any updates.");
+                return false;
+        }
+
         if (!path_is_absolute(c->parameter)) {
                 log_debug("Specified condition parameter '%s' is not absolute, assuming an update is needed.", c->parameter);
                 return true;

src/shared/install.c

@@ -376,6 +376,11 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
                                         verb, changes[i].path);
                         logged = true;
                         break;
+                case -EIDRM:
+                        log_error_errno(changes[i].type_or_errno, "Failed to %s unit, unit %s is a non-template unit.",
+                                        verb, changes[i].path);
+                        logged = true;
+                        break;
                 case -EUCLEAN:
                         log_error_errno(changes[i].type_or_errno,
                                         "Failed to %s unit, \"%s\" is not a valid unit name.",
@@ -1847,6 +1852,7 @@ static int install_info_symlink_wants(
                 size_t *n_changes) {
 
         _cleanup_free_ char *buf = NULL;
+        UnitNameFlags valid_dst_type = UNIT_NAME_ANY;
         const char *n;
         char **s;
         int r = 0, q;
@@ -1858,15 +1864,17 @@ static int install_info_symlink_wants(
         if (strv_isempty(list))
                 return 0;
 
-        if (unit_name_is_valid(i->name, UNIT_NAME_TEMPLATE)) {
+        if (unit_name_is_valid(i->name, UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE))
+                /* Not a template unit. Use the name directly. */
+                n = i->name;
+
+        else if (i->default_instance) {
                 UnitFileInstallInfo instance = {
                         .type = _UNIT_FILE_TYPE_INVALID,
                 };
                 _cleanup_free_ char *path = NULL;
 
-                /* If this is a template, and we have no instance, don't do anything */
-                if (!i->default_instance)
-                        return 1;
+                /* If this is a template, and we have a default instance, use it. */
 
                 r = unit_name_replace_instance(i->name, i->default_instance, &buf);
                 if (r < 0)
@@ -1885,8 +1893,14 @@ static int install_info_symlink_wants(
                 }
 
                 n = buf;
-        } else
+
+        } else {
+                /* We have a template, but no instance yet. When used with an instantiated unit, we will get
+                 * the instance from that unit. Cannot be used with non-instance units. */
+
+                valid_dst_type = UNIT_NAME_INSTANCE | UNIT_NAME_TEMPLATE;
                 n = i->name;
+        }
 
         STRV_FOREACH(s, list) {
                 _cleanup_free_ char *path = NULL, *dst = NULL;
@@ -1895,9 +1909,17 @@ static int install_info_symlink_wants(
                 if (q < 0)
                         return q;
 
-                if (!unit_name_is_valid(dst, UNIT_NAME_ANY)) {
-                        unit_file_changes_add(changes, n_changes, -EUCLEAN, dst, NULL);
-                        r = -EUCLEAN;
+                if (!unit_name_is_valid(dst, valid_dst_type)) {
+                        /* Generate a proper error here: EUCLEAN if the name is generally bad,
+                         * EIDRM if the template status doesn't match. */
+                        if (unit_name_is_valid(dst, UNIT_NAME_ANY)) {
+                                unit_file_changes_add(changes, n_changes, -EIDRM, dst, n);
+                                r = -EIDRM;
+                        } else {
+                                unit_file_changes_add(changes, n_changes, -EUCLEAN, dst, NULL);
+                                r = -EUCLEAN;
+                        }
+
                         continue;
                 }
 

src/udev/test-udev-node.c

@@ -33,6 +33,18 @@ static void test_udev_node_escape_path(void) {
         strcpy(b + sizeof(b) - 12, "N3YhcCqFeID");
 
         test_udev_node_escape_path_one(a, b);
+
+        strcpy(a + sizeof(a) - 12 - 9, "N3YhcCqFeID");
+        strcpy(b + sizeof(b) - 12, "L1oK9iKWdmi");
+        test_udev_node_escape_path_one(a, b);
+
+        strcpy(a + sizeof(a) - 12 - 9, "a");
+        strcpy(b + sizeof(b) - 12, "A7oaHBRuuZq");
+        test_udev_node_escape_path_one(a, b);
+
+        a[sizeof(a) - 12 - 9] = '\0';
+        b[sizeof(a) - 12] = '\0';
+        test_udev_node_escape_path_one(a, b);
 }
 
 int main(int argc, char *argv[]) {

src/udev/udev-node.c

@@ -219,20 +219,21 @@ size_t udev_node_escape_path(const char *src, char *dest, size_t size) {
 
         assert(src);
         assert(dest);
+        assert(size >= 12);
 
         for (i = 0, j = 0; src[i] != '\0'; i++) {
                 if (src[i] == '/') {
-                        if (j+4 >= size)
+                        if (j+4 >= size - 12 + 1)
                                 goto toolong;
                         memcpy(&dest[j], "\\x2f", 4);
                         j += 4;
                 } else if (src[i] == '\\') {
-                        if (j+4 >= size)
+                        if (j+4 >= size - 12 + 1)
                                 goto toolong;
                         memcpy(&dest[j], "\\x5c", 4);
                         j += 4;
                 } else {
-                        if (j+1 >= size)
+                        if (j+1 >= size - 12 + 1)
                                 goto toolong;
                         dest[j] = src[i];
                         j++;
@@ -247,8 +248,6 @@ toolong:
 
         h = siphash24_string(src, UDEV_NODE_HASH_KEY.bytes);
 
-        assert(size >= 12);
-
         for (unsigned k = 0; k <= 10; k++)
                 dest[size - k - 2] = urlsafe_base64char((h >> (k * 6)) & 63);
 

src/udev/udevadm-trigger.c

@@ -222,7 +222,7 @@ static int help(void) {
                "  -a --attr-match=FILE[=VALUE]      Trigger devices with a matching attribute\n"
                "  -A --attr-nomatch=FILE[=VALUE]    Exclude devices with a matching attribute\n"
                "  -p --property-match=KEY=VALUE     Trigger devices with a matching property\n"
-               "  -g --tag-match=KEY=VALUE          Trigger devices with a matching property\n"
+               "  -g --tag-match=TAG                Trigger devices with a matching tag\n"
                "  -y --sysname-match=NAME           Trigger devices with this /sys path\n"
                "     --name-match=NAME              Trigger devices with this /dev name\n"
                "  -b --parent-match=NAME            Trigger devices with that parent device\n"

units/proc-sys-fs-binfmt_misc.automount

@@ -13,6 +13,7 @@ Documentation=https://www.kernel.org/doc/html/latest/admin-guide/binfmt-misc.htm
 Documentation=https://www.freedesktop.org/wiki/Software/systemd/APIFileSystems
 DefaultDependencies=no
 Before=sysinit.target
+Conflicts=shutdown.target
 ConditionPathExists=/proc/sys/fs/binfmt_misc/
 ConditionPathIsReadWrite=/proc/sys/