Merge pull request #21270 from poettering/event-mem-corruption

sd-event: fix memory corruption

man/rules/meson.build

@@ -518,7 +518,9 @@ manpages = [
   ''],
  ['sd_event_add_inotify',
   '3',
-  ['sd_event_inotify_handler_t', 'sd_event_source_get_inotify_mask'],
+  ['sd_event_add_inotify_fd',
+   'sd_event_inotify_handler_t',
+   'sd_event_source_get_inotify_mask'],
   ''],
  ['sd_event_add_io',
   '3',

man/sd_event_add_inotify.xml

@@ -17,6 +17,7 @@
 
   <refnamediv>
     <refname>sd_event_add_inotify</refname>
+    <refname>sd_event_add_inotify_fd</refname>
     <refname>sd_event_source_get_inotify_mask</refname>
     <refname>sd_event_inotify_handler_t</refname>
 
@@ -46,6 +47,16 @@
         <paramdef>void *<parameter>userdata</parameter></paramdef>
       </funcprototype>
 
+      <funcprototype>
+        <funcdef>int <function>sd_event_add_inotify_fd</function></funcdef>
+        <paramdef>sd_event *<parameter>event</parameter></paramdef>
+        <paramdef>sd_event_source **<parameter>source</parameter></paramdef>
+        <paramdef>int <parameter>fd</parameter></paramdef>
+        <paramdef>uint32_t <parameter>mask</parameter></paramdef>
+        <paramdef>sd_event_inotify_handler_t <parameter>handler</parameter></paramdef>
+        <paramdef>void *<parameter>userdata</parameter></paramdef>
+      </funcprototype>
+
       <funcprototype>
         <funcdef>int <function>sd_event_source_get_inotify_mask</function></funcdef>
         <paramdef>sd_event_source *<parameter>source</parameter></paramdef>
@@ -71,6 +82,11 @@
     <citerefentry project='man-pages'><refentrytitle>inotify</refentrytitle><manvolnum>7</manvolnum></citerefentry> for
     further information.</para>
 
+    <para><function>sd_event_add_inotify_fd()</function> is identical to
+    <function>sd_event_add_inotify()</function>, except that it takes a file descriptor to an inode (possibly
+    an <constant>O_PATH</constant> one, but any other will do too) instead of a path in the file
+    system.</para>
+
     <para>If multiple event sources are installed for the same inode the backing inotify watch descriptor is
     automatically shared. The mask parameter may contain any flag defined by the inotify API, with the exception of
     <constant>IN_MASK_ADD</constant>.</para>
@@ -157,6 +173,19 @@
           <listitem><para>The passed event source is not an inotify process event source.</para></listitem>
         </varlistentry>
 
+        <varlistentry>
+          <term><constant>-EBADF</constant></term>
+
+          <listitem><para>The passed file descriptor is not valid.</para></listitem>
+        </varlistentry>
+
+        <varlistentry>
+          <term><constant>-ENOSYS</constant></term>
+
+          <listitem><para><function>sd_event_add_inotify_fd()</function> was called without
+          <filename>/proc/</filename> mounted.</para></listitem>
+        </varlistentry>
+
       </variablelist>
     </refsect2>
   </refsect1>

src/basic/inotify-util.c

@@ -2,14 +2,26 @@
 
 #include "fd-util.h"
 #include "inotify-util.h"
+#include "stat-util.h"
 
 int inotify_add_watch_fd(int fd, int what, uint32_t mask) {
-        int wd;
+        int wd, r;
 
         /* This is like inotify_add_watch(), except that the file to watch is not referenced by a path, but by an fd */
         wd = inotify_add_watch(fd, FORMAT_PROC_FD_PATH(what), mask);
-        if (wd < 0)
-                return -errno;
+        if (wd < 0) {
+                if (errno != ENOENT)
+                        return -errno;
+
+                /* Didn't work with ENOENT? If so, then either /proc/ isn't mounted, or the fd is bad */
+                r = proc_mounted();
+                if (r == 0)
+                        return -ENOSYS;
+                if (r > 0)
+                        return -EBADF;
+
+                return -ENOENT; /* OK, no clue, let's propagate the original error */
+        }
 
         return wd;
 }

src/journal/journald-stream.c

@@ -109,7 +109,6 @@ StdoutStream* stdout_stream_free(StdoutStream *s) {
                 return NULL;
 
         if (s->server) {
-
                 if (s->context)
                         client_context_release(s->server, s->context);
 
@@ -123,11 +122,7 @@ StdoutStream* stdout_stream_free(StdoutStream *s) {
                 (void) server_start_or_stop_idle_timer(s->server); /* Maybe we are idle now? */
         }
 
-        if (s->event_source) {
-                sd_event_source_set_enabled(s->event_source, SD_EVENT_OFF);
-                s->event_source = sd_event_source_unref(s->event_source);
-        }
-
+        sd_event_source_disable_unref(s->event_source);
         safe_close(s->fd);
         free(s->label);
         free(s->identifier);

src/libsystemd/libsystemd.sym

@@ -766,4 +766,5 @@ global:
 LIBSYSTEMD_250 {
 global:
         sd_device_get_diskseq;
+        sd_event_add_inotify_fd;
 } LIBSYSTEMD_249;

src/libsystemd/sd-bus/sd-bus.c

@@ -63,7 +63,6 @@
 
 static int bus_poll(sd_bus *bus, bool need_more, uint64_t timeout_usec);
 static void bus_detach_io_events(sd_bus *b);
-static void bus_detach_inotify_event(sd_bus *b);
 
 static thread_local sd_bus *default_system_bus = NULL;
 static thread_local sd_bus *default_user_bus = NULL;
@@ -140,7 +139,7 @@ void bus_close_io_fds(sd_bus *b) {
 void bus_close_inotify_fd(sd_bus *b) {
         assert(b);
 
-        bus_detach_inotify_event(b);
+        b->inotify_event_source = sd_event_source_disable_unref(b->inotify_event_source);
 
         b->inotify_fd = safe_close(b->inotify_fd);
         b->inotify_watches = mfree(b->inotify_watches);
@@ -3747,15 +3746,8 @@ int bus_attach_io_events(sd_bus *bus) {
 static void bus_detach_io_events(sd_bus *bus) {
         assert(bus);
 
-        if (bus->input_io_event_source) {
-                sd_event_source_set_enabled(bus->input_io_event_source, SD_EVENT_OFF);
-                bus->input_io_event_source = sd_event_source_unref(bus->input_io_event_source);
-        }
-
-        if (bus->output_io_event_source) {
-                sd_event_source_set_enabled(bus->output_io_event_source, SD_EVENT_OFF);
-                bus->output_io_event_source = sd_event_source_unref(bus->output_io_event_source);
-        }
+        bus->input_io_event_source = sd_event_source_disable_unref(bus->input_io_event_source);
+        bus->output_io_event_source = sd_event_source_disable_unref(bus->output_io_event_source);
 }
 
 int bus_attach_inotify_event(sd_bus *bus) {
@@ -3787,15 +3779,6 @@ int bus_attach_inotify_event(sd_bus *bus) {
         return 0;
 }
 
-static void bus_detach_inotify_event(sd_bus *bus) {
-        assert(bus);
-
-        if (bus->inotify_event_source) {
-                sd_event_source_set_enabled(bus->inotify_event_source, SD_EVENT_OFF);
-                bus->inotify_event_source = sd_event_source_unref(bus->inotify_event_source);
-        }
-}
-
 _public_ int sd_bus_attach_event(sd_bus *bus, sd_event *event, int priority) {
         int r;
 
@@ -3860,17 +3843,9 @@ _public_ int sd_bus_detach_event(sd_bus *bus) {
                 return 0;
 
         bus_detach_io_events(bus);
-        bus_detach_inotify_event(bus);
-
-        if (bus->time_event_source) {
-                sd_event_source_set_enabled(bus->time_event_source, SD_EVENT_OFF);
-                bus->time_event_source = sd_event_source_unref(bus->time_event_source);
-        }
-
-        if (bus->quit_event_source) {
-                sd_event_source_set_enabled(bus->quit_event_source, SD_EVENT_OFF);
-                bus->quit_event_source = sd_event_source_unref(bus->quit_event_source);
-        }
+        bus->inotify_event_source = sd_event_source_disable_unref(bus->inotify_event_source);
+        bus->time_event_source = sd_event_source_disable_unref(bus->time_event_source);
+        bus->quit_event_source = sd_event_source_disable_unref(bus->quit_event_source);
 
         bus->event = sd_event_unref(bus->event);
         return 1;

src/libsystemd/sd-event/event-source.h

@@ -214,6 +214,11 @@ struct inotify_data {
          * the events locally if they can't be coalesced). */
         unsigned n_pending;
 
+        /* If this counter is non-zero, don't GC the inotify data object even if not used to watch any inode
+         * anymore. This is useful to pin the object for a bit longer, after the last event source needing it
+         * is gone. */
+        unsigned n_busy;
+
         /* A linked list of all inotify objects with data already read, that still need processing. We keep this list
          * to make it efficient to figure out what inotify objects to process data on next. */
         LIST_FIELDS(struct inotify_data, buffered);