xdg-autostart-service: Fix binary escaping and simplify code a bit

Instead of escaping each component separately, we can instead use
quote_command_line. Doing so simplifies the code and fixes an issue
where spaces inside the executable name were not escaped.

Co-Authored-By: David Edmundson <kde@davidedmundson.co.uk>

src/basic/inotify-util.h

@@ -6,12 +6,31 @@
 #include <stddef.h>
 #include <sys/inotify.h>
 
+#include "log.h"
+
 #define INOTIFY_EVENT_MAX (offsetof(struct inotify_event, name) + NAME_MAX + 1)
 
+#define _FOREACH_INOTIFY_EVENT(e, buffer, sz, log_level, start, end)    \
+        for (struct inotify_event                                       \
+                            *start = &((buffer).ev),                    \
+                     *end = (struct inotify_event*) ((uint8_t*) start + (sz)), \
+                     *e = start;                                        \
+             (uint8_t*) e + sizeof(struct inotify_event) <= (uint8_t*) end && \
+             (uint8_t*) e + sizeof(struct inotify_event) + e->len <= (uint8_t*) end ? true : \
+                     ({                                                 \
+                             log_full(log_level, "Received invalid inotify event, ignoring."); \
+                             false;                                     \
+                     });                                                \
+             e = (struct inotify_event*) ((uint8_t*) e + sizeof(struct inotify_event) + e->len))
+
+#define _FOREACH_INOTIFY_EVENT_FULL(e, buffer, sz, log_level)           \
+        _FOREACH_INOTIFY_EVENT(e, buffer, sz, log_level, UNIQ_T(start, UNIQ), UNIQ_T(end, UNIQ))
+
 #define FOREACH_INOTIFY_EVENT(e, buffer, sz)                    \
-        for ((e) = &buffer.ev;                                \
+        _FOREACH_INOTIFY_EVENT_FULL(e, buffer, sz, LOG_DEBUG)
-             (uint8_t*) (e) < (uint8_t*) (buffer.raw) + (sz); \
+
-             (e) = (struct inotify_event*) ((uint8_t*) (e) + sizeof(struct inotify_event) + (e)->len))
+#define FOREACH_INOTIFY_EVENT_WARN(e, buffer, sz)               \
+        _FOREACH_INOTIFY_EVENT_FULL(e, buffer, sz, LOG_WARNING)
 
 union inotify_event_buffer {
         struct inotify_event ev;

src/basic/terminal-util.c

@@ -432,7 +432,6 @@ int acquire_terminal(
 
                 for (;;) {
                         union inotify_event_buffer buffer;
-                        struct inotify_event *e;
                         ssize_t l;
 
                         if (timeout != USEC_INFINITY) {

src/core/cgroup.c

@@ -3227,7 +3227,6 @@ static int on_cgroup_inotify_event(sd_event_source *s, int fd, uint32_t revents,
 
         for (;;) {
                 union inotify_event_buffer buffer;
-                struct inotify_event *e;
                 ssize_t l;
 
                 l = read(fd, &buffer, sizeof(buffer));
@@ -3238,7 +3237,7 @@ static int on_cgroup_inotify_event(sd_event_source *s, int fd, uint32_t revents,
                         return log_error_errno(errno, "Failed to read control group inotify events: %m");
                 }
 
-                FOREACH_INOTIFY_EVENT(e, buffer, l) {
+                FOREACH_INOTIFY_EVENT_WARN(e, buffer, l) {
                         Unit *u;
 
                         if (e->wd < 0)

src/core/path.c

@@ -173,7 +173,6 @@ void path_spec_unwatch(PathSpec *s) {
 
 int path_spec_fd_event(PathSpec *s, uint32_t revents) {
         union inotify_event_buffer buffer;
-        struct inotify_event *e;
         ssize_t l;
 
         assert(s);
@@ -191,7 +190,7 @@ int path_spec_fd_event(PathSpec *s, uint32_t revents) {
         }
 
         if (IN_SET(s->type, PATH_CHANGED, PATH_MODIFIED))
-                FOREACH_INOTIFY_EVENT(e, buffer, l)
+                FOREACH_INOTIFY_EVENT_WARN(e, buffer, l)
                         if (s->primary_wd == e->wd)
                                 return 1;
 

src/libsystemd/sd-journal/sd-journal.c

@@ -2677,7 +2677,6 @@ _public_ int sd_journal_process(sd_journal *j) {
 
         for (;;) {
                 union inotify_event_buffer buffer;
-                struct inotify_event *e;
                 ssize_t l;
 
                 l = read(j->inotify_fd, &buffer, sizeof(buffer));

src/libsystemd/sd-network/sd-network.c

@@ -469,7 +469,6 @@ sd_network_monitor* sd_network_monitor_unref(sd_network_monitor *m) {
 
 int sd_network_monitor_flush(sd_network_monitor *m) {
         union inotify_event_buffer buffer;
-        struct inotify_event *e;
         ssize_t l;
         int fd, k;
 

src/shared/efi-loader.c

@@ -62,8 +62,6 @@ int efi_loader_get_device_part_uuid(sd_id128_t *ret) {
         _cleanup_free_ char *p = NULL;
         int r, parsed[16];
 
-        assert(ret);
-
         if (!is_efi_boot())
                 return -EOPNOTSUPP;
 

src/timesync/wait-sync.c

@@ -80,7 +80,6 @@ static int inotify_handler(sd_event_source *s,
         sd_event *event = sd_event_source_get_event(s);
         ClockState *sp = userdata;
         union inotify_event_buffer buffer;
-        struct inotify_event *e;
         ssize_t l;
 
         l = read(fd, &buffer, sizeof(buffer));
@@ -90,7 +89,7 @@ static int inotify_handler(sd_event_source *s,
 
                 return log_warning_errno(errno, "Lost access to inotify: %m");
         }
-        FOREACH_INOTIFY_EVENT(e, buffer, l)
+        FOREACH_INOTIFY_EVENT_WARN(e, buffer, l)
                 process_inotify_event(event, sp, e);
 
         return 0;

src/udev/udevd.c

@@ -1334,7 +1334,6 @@ static int synthesize_change(sd_device *dev) {
 static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
         Manager *manager = userdata;
         union inotify_event_buffer buffer;
-        struct inotify_event *e;
         ssize_t l;
         int r;
 
@@ -1352,7 +1351,7 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda
                 return log_error_errno(errno, "Failed to read inotify fd: %m");
         }
 
-        FOREACH_INOTIFY_EVENT(e, buffer, l) {
+        FOREACH_INOTIFY_EVENT_WARN(e, buffer, l) {
                 _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
                 const char *devnode;
 

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

@@ -25,13 +25,13 @@ static void test_xdg_format_exec_start_one(const char *exec, const char *expecte
 }
 
 TEST(xdg_format_exec_start) {
-        test_xdg_format_exec_start_one("/bin/sleep 100", "/bin/sleep \"100\"");
+        test_xdg_format_exec_start_one("/bin/sleep 100", "/bin/sleep 100");
 
         /* All standardised % identifiers are stripped. */
         test_xdg_format_exec_start_one("/bin/sleep %f \"%F\" %u %U %d %D\t%n %N %i %c %k %v %m", "/bin/sleep");
 
         /* Unknown % identifier currently remain, but are escaped. */
-        test_xdg_format_exec_start_one("/bin/sleep %X \"%Y\"", "/bin/sleep \"%%X\" \"%%Y\"");
+        test_xdg_format_exec_start_one("/bin/sleep %X \"%Y\"", "/bin/sleep %%X %%Y");
 
         test_xdg_format_exec_start_one("/bin/sleep \";\\\"\"", "/bin/sleep \";\\\"\"");
 }

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

@@ -396,7 +396,7 @@ int xdg_autostart_format_exec_start(
 
         first_arg = true;
         for (i = n = 0; exec_split[i]; i++) {
-                _cleanup_free_ char *c = NULL, *raw = NULL, *p = NULL, *escaped = NULL, *quoted = NULL;
+                _cleanup_free_ char *c = NULL, *raw = NULL, *percent = NULL;
                 ssize_t l;
 
                 l = cunescape(exec_split[i], 0, &c);
@@ -412,11 +412,7 @@ int xdg_autostart_format_exec_start(
                         if (r < 0)
                                 return log_info_errno(r, "Exec binary '%s' does not exist: %m", c);
 
-                        escaped = cescape(executable);
+                        free_and_replace(exec_split[n++], executable);
-                        if (!escaped)
-                                return log_oom();
-
-                        free_and_replace(exec_split[n++], escaped);
                         continue;
                 }
 
@@ -445,23 +441,16 @@ int xdg_autostart_format_exec_start(
                 raw = strreplace(c, "%%", "%");
                 if (!raw)
                         return log_oom();
-                p = strreplace(raw, "%", "%%");
+                percent = strreplace(raw, "%", "%%");
-                if (!p)
+                if (!percent)
-                        return log_oom();
-                escaped = cescape(p);
-                if (!escaped)
                         return log_oom();
 
-                quoted = strjoin("\"", escaped, "\"");
+                free_and_replace(exec_split[n++], percent);
-                if (!quoted)
-                        return log_oom();
-
-                free_and_replace(exec_split[n++], quoted);
         }
         for (; exec_split[n]; n++)
                 exec_split[n] = mfree(exec_split[n]);
 
-        res = strv_join(exec_split, " ");
+        res = quote_command_line(exec_split, SHELL_ESCAPE_EMPTY);
         if (!res)
                 return log_oom();