1
0
mirror of https://github.com/systemd/systemd.git synced 2024-12-22 17:35:35 +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)
This commit is contained in:
Yu Watanabe 2024-04-22 05:22:24 +09:00 committed by Mike Yuan
parent 3ee5e3d046
commit 2fa480592d
3 changed files with 39 additions and 18 deletions

View File

@ -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()</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

@ -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;
}

View File

@ -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;