Merge pull request #17211 from poettering/udev-loop-fixes

two udev fixes, split out of #16859

src/basic/missing_syscall.h

@@ -15,6 +15,12 @@
 #include <asm/sgidefs.h>
 #endif
 
+#if defined(__x86_64__) && defined(__ILP32__)
+#define systemd_SC_arch_bias(x) ((x) | /* __X32_SYSCALL_BIT */ 0x40000000)
+#else
+#define systemd_SC_arch_bias(x) (x)
+#endif
+
 #include "missing_keyctl.h"
 #include "missing_stat.h"
 
@@ -34,7 +40,7 @@ static inline int missing_pivot_root(const char *new_root, const char *put_old)
 /* ======================================================================= */
 
 #if defined __x86_64__
-#  define systemd_NR_memfd_create 319
+#  define systemd_NR_memfd_create systemd_SC_arch_bias(319)
 #elif defined __arm__
 #  define systemd_NR_memfd_create 385
 #elif defined __aarch64__
@@ -91,7 +97,7 @@ static inline int missing_memfd_create(const char *name, unsigned int flags) {
 /* ======================================================================= */
 
 #if defined __x86_64__
-#  define systemd_NR_getrandom 318
+#  define systemd_NR_getrandom systemd_SC_arch_bias(318)
 #elif defined(__i386__)
 #  define systemd_NR_getrandom 355
 #elif defined(__arm__)
@@ -167,7 +173,7 @@ static inline pid_t missing_gettid(void) {
 /* ======================================================================= */
 
 #if defined(__x86_64__)
-#  define systemd_NR_name_to_handle_at 303
+#  define systemd_NR_name_to_handle_at systemd_SC_arch_bias(303)
 #elif defined(__i386__)
 #  define systemd_NR_name_to_handle_at 341
 #elif defined(__arm__)
@@ -224,7 +230,7 @@ static inline int missing_name_to_handle_at(int fd, const char *name, struct fil
 #elif defined __arm__
 #  define systemd_NR_setns 375
 #elif defined(__x86_64__)
-#  define systemd_NR_setns 308
+#  define systemd_NR_setns systemd_SC_arch_bias(308)
 #elif defined(__i386__)
 #  define systemd_NR_setns 346
 #elif defined(__powerpc__)
@@ -277,7 +283,7 @@ static inline pid_t raw_getpid(void) {
 /* ======================================================================= */
 
 #if defined __x86_64__
-#  define systemd_NR_renameat2 316
+#  define systemd_NR_renameat2 systemd_SC_arch_bias(316)
 #elif defined __arm__
 #  define systemd_NR_renameat2 382
 #elif defined __aarch64__
@@ -386,7 +392,7 @@ static inline key_serial_t missing_request_key(const char *type, const char *des
 /* ======================================================================= */
 
 #if defined(__x86_64__)
-#  define systemd_NR_copy_file_range 326
+#  define systemd_NR_copy_file_range systemd_SC_arch_bias(326)
 #elif defined(__i386__)
 #  define systemd_NR_copy_file_range 377
 #elif defined __s390__
@@ -438,7 +444,7 @@ static inline ssize_t missing_copy_file_range(int fd_in, loff_t *off_in,
 #if defined __i386__
 #  define systemd_NR_bpf 357
 #elif defined __x86_64__
-#  define systemd_NR_bpf 321
+#  define systemd_NR_bpf systemd_SC_arch_bias(321)
 #elif defined __aarch64__
 #  define systemd_NR_bpf 280
 #elif defined __arm__
@@ -490,7 +496,7 @@ static inline int missing_bpf(int cmd, union bpf_attr *attr, size_t size) {
 #  if defined __i386__
 #    define systemd_NR_pkey_mprotect 380
 #  elif defined __x86_64__
-#    define systemd_NR_pkey_mprotect 329
+#    define systemd_NR_pkey_mprotect systemd_SC_arch_bias(329)
 #  elif defined __aarch64__
 #    define systemd_NR_pkey_mprotect 288
 #  elif defined __arm__
@@ -543,7 +549,7 @@ assert_cc(__NR_pkey_mprotect == systemd_NR_pkey_mprotect);
 #elif defined __sparc__
 #  define systemd_NR_statx 360
 #elif defined __x86_64__
-#  define systemd_NR_statx 332
+#  define systemd_NR_statx systemd_SC_arch_bias(332)
 #else
 #  warning "statx() syscall number unknown for your architecture"
 #endif
@@ -643,7 +649,7 @@ static inline long missing_get_mempolicy(int *mode, unsigned long *nodemask,
 #elif defined __ia64__
 #  define systemd_NR_pidfd_send_signal (424 + 1024)
 #else
-#  define systemd_NR_pidfd_send_signal 424
+#  define systemd_NR_pidfd_send_signal systemd_SC_arch_bias(424)
 #endif
 
 /* may be (invalid) negative number due to libseccomp, see PR 13319 */
@@ -687,7 +693,7 @@ static inline int missing_pidfd_send_signal(int fd, int sig, siginfo_t *info, un
 #elif defined __ia64__
 #  define systemd_NR_pidfd_open (434 + 1024)
 #else
-#  define systemd_NR_pidfd_open 434
+#  define systemd_NR_pidfd_open systemd_SC_arch_bias(434)
 #endif
 
 /* may be (invalid) negative number due to libseccomp, see PR 13319 */

src/network/networkd-route.c

@@ -594,6 +594,7 @@ int route_remove(
 
         if (!manager)
                 manager = link->manager;
+        /* link may be NULL! */
 
         r = sd_rtnl_message_new_route(manager->rtnl, &req,
                                       RTM_DELROUTE, route->family,
@@ -676,6 +677,8 @@ int route_remove(
                 return log_link_error_errno(link, r, "Could not append RTA_PRIORITY attribute: %m");
 
         if (!IN_SET(route->type, RTN_UNREACHABLE, RTN_PROHIBIT, RTN_BLACKHOLE, RTN_THROW)) {
+                assert(link); /* Those routes must be attached to a specific link */
+
                 r = sd_netlink_message_append_u32(req, RTA_OIF, link->ifindex);
                 if (r < 0)
                         return log_link_error_errno(link, r, "Could not append RTA_OIF attribute: %m");
@@ -687,8 +690,7 @@ int route_remove(
         if (r < 0)
                 return log_link_error_errno(link, r, "Could not send rtnetlink message: %m");
 
-        if (link)
-                link_ref(link);
+        link_ref(link); /* link may be NULL, link_ref() is OK with that */
 
         return 0;
 }

src/shared/udev-util.c

@@ -154,6 +154,22 @@ static int device_monitor_handler(sd_device_monitor *monitor, sd_device *device,
         assert(data->sysname || data->devlink);
         assert(!data->device);
 
+        /* Ignore REMOVE events here. We are waiting for initialization after all, not de-initialization. We
+         * might see a REMOVE event from an earlier use of the device (devices by the same name are recycled
+         * by the kernel after all), which we should not get confused by. After all we cannot distinguish use
+         * cycles of the devices, as the udev queue is entirely asynchronous.
+         *
+         * If we see a REMOVE event here for the use cycle we actually care about then we won't notice of
+         * course, but that should be OK, given the timeout logic used on the wait loop: this will be noticed
+         * by means of -ETIMEDOUT. Thus we won't notice immediately, but eventually, and that should be
+         * sufficient for an error path that should regularly not happen.
+         *
+         * (And yes, we only need to special case REMOVE. It's the only "negative" event type, where a device
+         * ceases to exist. All other event types are "positive": the device exists and is registered in the
+         * udev database, thus whenever we see the event, we can consider it initialized.) */
+        if (device_for_action(device, DEVICE_ACTION_REMOVE))
+                return 0;
+
         if (data->sysname && sd_device_get_sysname(device, &sysname) >= 0 && streq(sysname, data->sysname))
                 goto found;
 

src/systemctl/systemctl-edit.c

@@ -18,7 +18,7 @@
 #include "tmpfile-util.h"
 
 int cat(int argc, char *argv[], void *userdata) {
-        _cleanup_(hashmap_freep) Hashmap *cached_id_map = NULL, *cached_name_map = NULL;
+        _cleanup_(hashmap_freep) Hashmap *cached_name_map = NULL, *cached_id_map = NULL;
         _cleanup_(lookup_paths_free) LookupPaths lp = {};
         _cleanup_strv_free_ char **names = NULL;
         char **name;
@@ -55,7 +55,7 @@ int cat(int argc, char *argv[], void *userdata) {
                 _cleanup_free_ char *fragment_path = NULL;
                 _cleanup_strv_free_ char **dropin_paths = NULL;
 
-                r = unit_find_paths(bus, *name, &lp, false, &cached_id_map, &cached_name_map, &fragment_path, &dropin_paths);
+                r = unit_find_paths(bus, *name, &lp, false, &cached_name_map, &cached_id_map, &fragment_path, &dropin_paths);
                 if (r == -ERFKILL) {
                         printf("%s# Unit %s is masked%s.\n",
                                ansi_highlight_magenta(),
@@ -318,7 +318,7 @@ static int run_editor(char **paths) {
 }
 
 static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) {
-        _cleanup_(hashmap_freep) Hashmap *cached_id_map = NULL, *cached_name_map = NULL;
+        _cleanup_(hashmap_freep) Hashmap *cached_name_map = NULL, *cached_id_map = NULL;
         _cleanup_(lookup_paths_free) LookupPaths lp = {};
         char **name;
         int r;
@@ -334,12 +334,12 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) {
                 _cleanup_free_ char *path = NULL, *new_path = NULL, *tmp_path = NULL, *tmp_name = NULL;
                 const char *unit_name;
 
-                r = unit_find_paths(bus, *name, &lp, false, &cached_id_map, &cached_name_map, &path, NULL);
+                r = unit_find_paths(bus, *name, &lp, false, &cached_name_map, &cached_id_map, &path, NULL);
                 if (r == -EKEYREJECTED) {
                         /* If loading of the unit failed server side complete, then the server won't tell us
                          * the unit file path. In that case, find the file client side. */
                         log_debug_errno(r, "Unit '%s' was not loaded correctly, retrying client-side.", *name);
-                        r = unit_find_paths(bus, *name, &lp, true, &cached_id_map, &cached_name_map, &path, NULL);
+                        r = unit_find_paths(bus, *name, &lp, true, &cached_name_map, &cached_id_map, &path, NULL);
                 }
                 if (r == -ERFKILL)
                         return log_error_errno(r, "Unit '%s' masked, cannot edit.", *name);

src/udev/udevd.c

@@ -459,6 +459,43 @@ static int worker_process_device(Manager *manager, sd_device *dev) {
                 return -ENOMEM;
 
         r = worker_lock_block_device(dev, &fd_lock);
+        if (r == -EAGAIN) {
+                /* So this is a block device and the device is locked currently via the BSD advisory locks —
+                 * someone else is exclusively using it. This means we don't run our udev rules now, to not
+                 * interfere. However we want to know when the device is unlocked again, and retrigger the
+                 * device again then, so that the rules are run eventually. For that we use IN_CLOSE_WRITE
+                 * inotify watches (which isn't exactly the same as waiting for the BSD locks to release, but
+                 * not totally off, as long as unlock+close() is done together, as it usually is).
+                 *
+                 * (The user-facing side of this: https://systemd.io/BLOCK_DEVICE_LOCKING)
+                 *
+                 * There's a bit of a chicken and egg problem here for this however: inotify watching is
+                 * supposed to be enabled via an option set via udev rules (OPTIONS+="watch"). If we skip the
+                 * udev rules here however (as we just said we do), we would thus never see that specific
+                 * udev rule, and thus never turn on inotify watching. But in order to catch up eventually
+                 * and run them we we need the inotify watching: hence a classic chicken and egg problem.
+                 *
+                 * Our way out here: if we see the block device locked, unconditionally watch the device via
+                 * inotify, regardless of any explicit request via OPTIONS+="watch". Thus, a device that is
+                 * currently locked via the BSD file locks will be treated as if we ran a single udev rule
+                 * only for it: the one that turns on inotify watching for it. If we eventually see the
+                 * inotify IN_CLOSE_WRITE event, and then run the rules after all and we then realize that
+                 * this wasn't actually requested (i.e. no OPTIONS+="watch" set) we'll simply turn off the
+                 * watching again (see below). Effectively this means: inotify watching is now enabled either
+                 * a) when the udev rules say so, or b) while the device is locked.
+                 *
+                 * Worst case scenario hence: in the (unlikely) case someone locked the device and we clash
+                 * with that we might do inotify watching for a brief moment for a device where we actually
+                 * weren't supposed to. But that shouldn't be too bad, in particular as BSD locks being taken
+                 * on a block device is kinda an indication that the inotify logic is desired too, to some
+                 * degree — they go hand-in-hand after all. */
+
+                log_device_debug(dev, "Block device is currently locked, installing watch to wait until the lock is released.");
+                (void) udev_watch_begin(dev);
+
+                /* Now the watch is installed, let's lock the device again, maybe in the meantime things changed */
+                r = worker_lock_block_device(dev, &fd_lock);
+        }
         if (r < 0)
                 return r;
 
@@ -475,13 +512,14 @@ static int worker_process_device(Manager *manager, sd_device *dev) {
                 /* in case rtnl was initialized */
                 manager->rtnl = sd_netlink_ref(udev_event->rtnl);
 
-        /* apply/restore inotify watch */
+        /* apply/restore/end inotify watch */
         if (udev_event->inotify_watch) {
                 (void) udev_watch_begin(dev);
                 r = device_update_db(dev);
                 if (r < 0)
                         return log_device_debug_errno(dev, r, "Failed to update database under /run/udev/data/: %m");
-        }
+        } else
+                (void) udev_watch_end(dev);
 
         log_device_debug(dev, "Device (SEQNUM=%"PRIu64", ACTION=%s) processed",
                          seqnum, device_action_to_string(action));

src/xdg-autostart-generator/xdg-autostart-service.c

@@ -28,6 +28,7 @@ XdgAutostartService* xdg_autostart_service_free(XdgAutostartService *s) {
 
         free(s->type);
         free(s->exec_string);
+        free(s->working_directory);
 
         strv_free(s->only_show_in);
         strv_free(s->not_show_in);
@@ -321,6 +322,7 @@ XdgAutostartService *xdg_autostart_service_parse_desktop(const char *path) {
         const ConfigTableItem items[] = {
                 { "Desktop Entry", "Name",                      xdg_config_parse_string, 0, &service->description},
                 { "Desktop Entry", "Exec",                      xdg_config_parse_string, 0, &service->exec_string},
+                { "Desktop Entry", "Path",                      xdg_config_parse_string, 0, &service->working_directory},
                 { "Desktop Entry", "TryExec",                   xdg_config_parse_string, 0, &service->try_exec},
                 { "Desktop Entry", "Type",                      xdg_config_parse_string, 0, &service->type},
                 { "Desktop Entry", "OnlyShowIn",                xdg_config_parse_strv, 0,   &service->only_show_in},
@@ -338,9 +340,12 @@ XdgAutostartService *xdg_autostart_service_parse_desktop(const char *path) {
                 { "Desktop Entry", "GenericName", NULL, 0, NULL},
                 { "Desktop Entry", "Icon", NULL, 0, NULL},
                 { "Desktop Entry", "Keywords", NULL, 0, NULL},
+                { "Desktop Entry", "MimeType", NULL, 0, NULL},
                 { "Desktop Entry", "NoDisplay", NULL, 0, NULL},
                 { "Desktop Entry", "StartupNotify", NULL, 0, NULL},
+                { "Desktop Entry", "StartupWMClass", NULL, 0, NULL},
                 { "Desktop Entry", "Terminal", NULL, 0, NULL},
+                { "Desktop Entry", "URL", NULL, 0, NULL},
                 { "Desktop Entry", "Version", NULL, 0, NULL},
                 {}
         };
@@ -606,6 +611,16 @@ int xdg_autostart_service_generate_unit(
                 "Slice=app.slice\n",
                 exec_start);
 
+        if (service->working_directory) {
+                _cleanup_free_ char *e_working_directory = NULL;
+
+                e_working_directory = cescape(service->working_directory);
+                if (!e_working_directory)
+                        return log_oom();
+
+                fprintf(f, "WorkingDirectory=-%s\n", e_working_directory);
+        }
+
         /* Generate an ExecCondition to check $XDG_CURRENT_DESKTOP */
         if (!strv_isempty(service->only_show_in) || !strv_isempty(service->not_show_in)) {
                 _cleanup_free_ char *only_show_in = NULL, *not_show_in = NULL, *e_only_show_in = NULL, *e_not_show_in = NULL;

src/xdg-autostart-generator/xdg-autostart-service.h

@@ -10,6 +10,7 @@ typedef struct XdgAutostartService {
 
         char *type; /* Purely as an assertion check */
         char *exec_string;
+        char *working_directory;
 
         char **only_show_in;
         char **not_show_in;