1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
|
From 2c30104f8344406e71b792a8691af60af3afe177 Mon Sep 17 00:00:00 2001
From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Tue, 2 Jul 2024 09:55:57 +0800
Subject: [PATCH] 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 2fa4805)
(cherry picked from commit 6d2dd43)
(cherry picked from commit 5f8cf63)
Conflict:test case adaptation
Reference:https://github.com/systemd/systemd-stable/commit/a4bb56c61a7bfef9bab3380b3c18709ab8fef3d8
---
man/sd_event_add_io.xml | 24 ++++++++++++++----------
src/libsystemd/sd-event/sd-event.c | 17 ++++++++---------
src/libsystemd/sd-event/test-event.c | 18 ++++++++++++++++++
3 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/man/sd_event_add_io.xml b/man/sd_event_add_io.xml
index da0fa58..9d4fd27 100644
--- 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 d53a7a1..0b59f63 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -2696,7 +2696,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);
@@ -2706,16 +2706,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);
@@ -2727,6 +2723,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 63d3ee7..695b0ed 100644
--- a/src/libsystemd/sd-event/test-event.c
+++ b/src/libsystemd/sd-event/test-event.c
@@ -809,6 +809,24 @@ 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]);
+}
+
TEST(fork) {
_cleanup_(sd_event_unrefp) sd_event *e = NULL;
int r;
--
2.27.0
|
