sd-event: fix fd leak when fd is owned by IO event source

When an IO event source owns relevant fd, replacing with a new fd leaks
the previously assigned fd.
===
sd_event_add_io(event, &s, fd, ...);
sd_event_source_set_io_fd_own(s, true);
sd_event_source_set_io_fd(s, new_fd);  <-- The previous fd is not closed.
sd_event_source_unref(s);  <-- new_fd is closed as expected.
===

Without the change, valgrind reports the leak:
==998589==
==998589== FILE DESCRIPTORS: 4 open (3 std) at exit.
==998589== Open file descriptor 4:
==998589==    at 0x4F119AB: pipe2 (in /usr/lib64/libc.so.6)
==998589==    by 0x408830: test_sd_event_source_set_io_fd (test-event.c:862)
==998589==    by 0x403302: run_test_table (tests.h:171)
==998589==    by 0x408E31: main (test-event.c:935)
==998589==
==998589==
==998589== HEAP SUMMARY:
==998589==     in use at exit: 0 bytes in 0 blocks
==998589==   total heap usage: 33,305 allocs, 33,305 frees, 1,283,581 bytes allocated
==998589==
==998589== All heap blocks were freed -- no leaks are possible
==998589==
==998589== For lists of detected and suppressed errors, rerun with: -s
==998589== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

diff --git a/man/sd_event_add_io.xml b/man/sd_event_add_io.xml
index 3935141fc0501c44bba9baa09869b4dea332a76c..09a5b11dfffe93046f279db2dd178f223d988c86 100644 (file)
--- a/man/sd_event_add_io.xml
+++ b/man/sd_event_add_io.xml
@@ -216,16 +216,20 @@
     source object and returns the non-negative file descriptor
     or a negative error number on error (see below).</para>
 
-    <para><function>sd_event_source_set_io_fd()</function>
-    changes the UNIX file descriptor of an I/O event source created
-    previously with <function>sd_event_add_io()</function>. It takes
-    the event source object and the new file descriptor.</para>
-
-    <para><function>sd_event_source_set_io_fd_own()</function> controls whether the file descriptor of the event source
-    shall be closed automatically when the event source is freed, i.e. whether it shall be considered 'owned' by the
-    event source object. By default it is not closed automatically, and the application has to do this on its own. The
-    <parameter>b</parameter> parameter is a boolean parameter: if zero, the file descriptor is not closed automatically
-    when the event source is freed, otherwise it is closed.</para>
+    <para><function>sd_event_source_set_io_fd()</function> changes the UNIX file descriptor of an I/O event
+    source created previously with <function>sd_event_add_io()</function>. It takes the event source object
+    and the new file descriptor. If the event source takes the ownership of the previous file descriptor,
+    that is, <function>sd_event_source_set_io_fd_own()</function> was called for the event source with a
+    non-zero value, then the previous file descriptor will be closed and the event source will also take the
+    ownership of the new file descriptor on success.</para>
+
+    <para><function>sd_event_source_set_io_fd_own()</function> controls whether the file descriptor of the
+    event source shall be closed automatically when the event source is freed (or when the file descriptor
+    assigned to the event source is replaced by <function>sd_event_source_set_io_fd()</function>), i.e.
+    whether it shall be considered 'owned' by the event source object. By default it is not closed
+    automatically, and the application has to do this on its own. The <parameter>b</parameter> parameter is a
+    boolean parameter: if zero, the file descriptor is not closed automatically when the event source is
+    freed, otherwise it is closed.</para>
 
     <para><function>sd_event_source_get_io_fd_own()</function> may be used to query the current setting of the file
     descriptor ownership boolean flag as set with <function>sd_event_source_set_io_fd_own()</function>. It returns

diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 39c60297f06280e11f15c44571dcaae4181d7238..bd5bd81ac4314bb2dcf2a10bffe497ff5d71625d 100644 (file)
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -2647,7 +2647,7 @@ _public_ int sd_event_source_get_io_fd(sd_event_source *s) {
 }
 
 _public_ int sd_event_source_set_io_fd(sd_event_source *s, int fd) {
-        int r;
+        int saved_fd, r;
 
         assert_return(s, -EINVAL);
         assert_return(fd >= 0, -EBADF);
@@ -2657,16 +2657,12 @@ _public_ int sd_event_source_set_io_fd(sd_event_source *s, int fd) {
         if (s->io.fd == fd)
                 return 0;
 
-        if (event_source_is_offline(s)) {
-                s->io.fd = fd;
-                s->io.registered = false;
-        } else {
-                int saved_fd;
+        saved_fd = s->io.fd;
+        s->io.fd = fd;
 
-                saved_fd = s->io.fd;
-                assert(s->io.registered);
+        assert(event_source_is_offline(s) == !s->io.registered);
 
-                s->io.fd = fd;
+        if (s->io.registered) {
                 s->io.registered = false;
 
                 r = source_io_register(s, s->enabled, s->io.events);
@@ -2679,6 +2675,9 @@ _public_ int sd_event_source_set_io_fd(sd_event_source *s, int fd) {
                 (void) epoll_ctl(s->event->epoll_fd, EPOLL_CTL_DEL, saved_fd, NULL);
         }
 
+        if (s->io.owned)
+                safe_close(saved_fd);
+
         return 0;
 }
 

diff --git a/src/libsystemd/sd-event/test-event.c b/src/libsystemd/sd-event/test-event.c
index 991aa925a42a7d125ac1b9da386fbe099fb1ecba..c617935992245d5426fc97ccafe35ffa7b3a9259 100644 (file)
--- a/src/libsystemd/sd-event/test-event.c
+++ b/src/libsystemd/sd-event/test-event.c
@@ -843,6 +843,24 @@ TEST(fork) {
         assert_se(r >= 0);
 }
 
+TEST(sd_event_source_set_io_fd) {
+        _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL;
+        _cleanup_(sd_event_unrefp) sd_event *e = NULL;
+        _cleanup_close_pair_ int pfd_a[2] = EBADF_PAIR, pfd_b[2] = EBADF_PAIR;
+
+        assert_se(sd_event_default(&e) >= 0);
+
+        assert_se(pipe2(pfd_a, O_CLOEXEC) >= 0);
+        assert_se(pipe2(pfd_b, O_CLOEXEC) >= 0);
+
+        assert_se(sd_event_add_io(e, &s, pfd_a[0], EPOLLIN, NULL, INT_TO_PTR(-ENOANO)) >= 0);
+        assert_se(sd_event_source_set_io_fd_own(s, true) >= 0);
+        TAKE_FD(pfd_a[0]);
+
+        assert_se(sd_event_source_set_io_fd(s, pfd_b[0]) >= 0);
+        TAKE_FD(pfd_b[0]);
+}
+
 static int hup_callback(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
         unsigned *c = userdata;