1
1
mirror of https://github.com/systemd/systemd-stable.git synced 2025-03-12 08:58:20 +03:00

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)

(cherry picked from commit 2fa480592d4f4334881361c5558f563e5ea4c9c3)
(cherry picked from commit 6d2dd436429aafcbb3fd8c99f6b69c9a108bf7f9)
(cherry picked from commit 5f8cf63f17c2ab4fecb0e65e6231ae6931270893)
(cherry picked from commit a4bb56c61a7bfef9bab3380b3c18709ab8fef3d8)
This commit is contained in:
Yu Watanabe 2024-04-22 05:22:24 +09:00 committed by Luca Boccassi
parent 8abd9814d5
commit b766a6b466
3 changed files with 39 additions and 18 deletions

View File

@ -217,16 +217,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()</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, 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_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

View File

@ -2255,7 +2255,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);
@ -2265,16 +2265,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);
@ -2287,6 +2283,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;
}

View File

@ -809,4 +809,22 @@ TEST(inotify_process_buffered_data) {
assert_se(sd_event_wait(e, 0) == 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, -EBADF }, pfd_b[2] = { -EBADF, -EBADF };
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]);
}
DEFINE_TEST_MAIN(LOG_DEBUG);