SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

json-stream: do not take ownership of passed fds on failure

Browse Source 
This makes json_stream_attach_fds() and json_stream_connect_fd_pair()
leave the passed file descriptors with the caller on failure. Callers
should call TAKE_FD() only after a successful call.

Also make qmp_client_connect_fd() always take ownership of the passed
file descriptor, including on failure. Previously, ownership of the
passed file descriptor on failure was unclear and depended on where
the function failed.
This commit is contained in:
Yu Watanabe
2026-08-14 23:55:51 +09:00
parent dc5de3fc03
commit 0bd40fdc07
6 changed files with 46 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libsystemd/sd-json/json-stream.c
+22 -17
View File
@@ -199,7 +199,6 @@ int json_stream_connect_address(JsonStream *s, const char *address) {
r = connect_unix_path(sock_fd, AT_FDCWD, address);
} else
r = RET_NERRNO(connect(sock_fd, &sockaddr.sa, r));
if (r < 0) {
if (!IN_SET(r, -EAGAIN, -EINPROGRESS))
return json_stream_log_errno(s, r, "Failed to connect to %s: %m", address);
@@ -212,8 +211,12 @@ int json_stream_connect_address(JsonStream *s, const char *address) {
s->flags |= JSON_STREAM_CONNECTING;
}
int fd = TAKE_FD(sock_fd);
return json_stream_attach_fds(s, fd, fd);
r = json_stream_attach_fds(s, sock_fd, sock_fd);
if (r < 0)
return r;
TAKE_FD(sock_fd);
return 0;
}
int json_stream_attach_fds(JsonStream *s, int input_fd, int output_fd) {
@@ -227,17 +230,9 @@ int json_stream_attach_fds(JsonStream *s, int input_fd, int output_fd) {
assert(output_fd != s->input_fd);
assert(output_fd != s->output_fd);
/* NB: input_fd and output_fd are donated to the JsonStream instance! */
/* NB: input_fd and output_fd are donated to the JsonStream instance on success. */
if (s->input_fd != s->output_fd) {
safe_close(s->input_fd);
safe_close(s->output_fd);
} else
safe_close(s->input_fd);
s->input_fd = input_fd;
s->output_fd = output_fd;
s->flags &= ~(JSON_STREAM_PREFER_READ|JSON_STREAM_PREFER_WRITE);
JsonStreamFlags flags = s->flags & ~(JSON_STREAM_PREFER_READ|JSON_STREAM_PREFER_WRITE);
/* Detect non-socket fds up front so the read/write paths use read()/write() for
* non-socket fds and send()/recv() for sockets (mostly for MSG_NOSIGNAL). */
@@ -245,17 +240,24 @@ int json_stream_attach_fds(JsonStream *s, int input_fd, int output_fd) {
if (fstat(input_fd, &st) < 0)
return -errno;
if (!S_ISSOCK(st.st_mode))
s->flags |= JSON_STREAM_PREFER_READ;
flags |= JSON_STREAM_PREFER_READ;
}
if (output_fd >= 0 && output_fd != input_fd) {
if (fstat(output_fd, &st) < 0)
return -errno;
if (!S_ISSOCK(st.st_mode))
s->flags |= JSON_STREAM_PREFER_WRITE;
} else if (FLAGS_SET(s->flags, JSON_STREAM_PREFER_READ))
s->flags |= JSON_STREAM_PREFER_WRITE;
flags |= JSON_STREAM_PREFER_WRITE;
} else if (FLAGS_SET(flags, JSON_STREAM_PREFER_READ))
flags |= JSON_STREAM_PREFER_WRITE;
if (s->input_fd != s->output_fd)
safe_close(s->output_fd);
safe_close(s->input_fd);
s->input_fd = input_fd;
s->output_fd = output_fd;
s->flags = flags;
return 0;
}
@@ -266,6 +268,9 @@ int json_stream_connect_fd_pair(JsonStream *s, int input_fd, int output_fd) {
assert(input_fd >= 0);
assert(output_fd >= 0);
/* NB: input_fd and output_fd are donated to the JsonStream instance on success.
* The fds may be changed to non-blocking mode even on failure. */
r = fd_nonblock(input_fd, true);
if (r < 0)
return json_stream_log_errno(s, r, "Failed to make input fd %d nonblocking: %m", input_fd);
src/libsystemd/sd-varlink/sd-varlink.c
+10 -5
View File
@@ -307,11 +307,12 @@ _public_ int sd_varlink_connect_exec(sd_varlink **ret, const char *_command, cha
if (r < 0)
return log_debug_errno(r, "Failed to create varlink object: %m");
int conn_fd = TAKE_FD(client_fd);
r = json_stream_attach_fds(&v->stream, conn_fd, conn_fd);
r = json_stream_attach_fds(&v->stream, client_fd, client_fd);
if (r < 0)
return r;
TAKE_FD(client_fd);
v->exec_pidref = TAKE_PIDREF(pidref);
varlink_set_state(v, VARLINK_IDLE_CLIENT);
@@ -395,11 +396,12 @@ static int varlink_connect_ssh_unix(sd_varlink **ret, const char *where) {
if (r < 0)
return log_debug_errno(r, "Failed to create varlink object: %m");
int conn_fd = TAKE_FD(client_fd);
r = json_stream_attach_fds(&v->stream, conn_fd, conn_fd);
r = json_stream_attach_fds(&v->stream, client_fd, client_fd);
if (r < 0)
return r;
TAKE_FD(client_fd);
v->exec_pidref = TAKE_PIDREF(pidref);
varlink_set_state(v, VARLINK_IDLE_CLIENT);
@@ -490,10 +492,13 @@ static int varlink_connect_ssh_exec(sd_varlink **ret, const char *where) {
if (r < 0)
return log_debug_errno(r, "Failed to create varlink object: %m");
r = json_stream_attach_fds(&v->stream, TAKE_FD(output_pipe[0]), TAKE_FD(input_pipe[1]));
r = json_stream_attach_fds(&v->stream, output_pipe[0], input_pipe[1]);
if (r < 0)
return r;
TAKE_FD(output_pipe[0]);
TAKE_FD(input_pipe[1]);
v->exec_pidref = TAKE_PIDREF(pidref);
varlink_set_state(v, VARLINK_IDLE_CLIENT);
src/shared/qmp-client.c
+6 -2
View File
@@ -655,10 +655,12 @@ static int qmp_client_capabilities_reply(
int qmp_client_connect_fd(QmpClient **ret, int fd) {
_cleanup_(qmp_client_unrefp) QmpClient *c = NULL;
_cleanup_close_ int fd_close = ASSERT_FD(TAKE_FD(fd));
int r;
assert(ret);
assert(fd >= 0);
/* This function always takes ownership of the passed fd. */
c = new(QmpClient, 1);
if (!c)
@@ -681,10 +683,12 @@ int qmp_client_connect_fd(QmpClient **ret, int fd) {
if (r < 0)
return r;
r = json_stream_connect_fd_pair(&c->stream, fd, fd);
r = json_stream_connect_fd_pair(&c->stream, fd_close, fd_close);
if (r < 0)
return r;
TAKE_FD(fd_close);
/* Eagerly queue qmp_capabilities. QEMU accepts commands as soon as the socket opens
* — its greeting is informational and doesn't gate writes on our side. FIFO ordering
* of the output queue guarantees cap precedes any user command a later invoke()
src/test/test-qmp-client-qemu.c
+3 -6
View File
@@ -123,12 +123,11 @@ TEST(qmp_client_qemu_handshake_and_schema) {
ASSERT_OK(start_qemu(qemu, qmp_fds[1], &pidref));
qmp_fds[1] = safe_close(qmp_fds[1]);
r = qmp_client_connect_fd(&client, qmp_fds[0]);
r = qmp_client_connect_fd(&client, TAKE_FD(qmp_fds[0]));
if (r < 0) {
log_tests_skipped_errno(r, "QMP connect failed (QEMU may not support -machine none)");
return;
}
TAKE_FD(qmp_fds[0]);
ASSERT_OK(qmp_client_attach_event(client, event, SD_EVENT_PRIORITY_NORMAL));
@@ -187,12 +186,11 @@ TEST(qmp_client_qemu_query_status) {
ASSERT_OK(start_qemu(qemu, qmp_fds[1], &pidref));
qmp_fds[1] = safe_close(qmp_fds[1]);
r = qmp_client_connect_fd(&client, qmp_fds[0]);
r = qmp_client_connect_fd(&client, TAKE_FD(qmp_fds[0]));
if (r < 0) {
log_tests_skipped_errno(r, "QMP connect failed");
return;
}
TAKE_FD(qmp_fds[0]);
ASSERT_OK(qmp_client_attach_event(client, event, SD_EVENT_PRIORITY_NORMAL));
@@ -277,12 +275,11 @@ TEST(qmp_client_qemu_add_fd) {
ASSERT_OK(start_qemu(qemu, qmp_fds[1], &pidref));
qmp_fds[1] = safe_close(qmp_fds[1]);
r = qmp_client_connect_fd(&client, qmp_fds[0]);
r = qmp_client_connect_fd(&client, TAKE_FD(qmp_fds[0]));
if (r < 0) {
log_tests_skipped_errno(r, "QMP connect failed");
return;
}
TAKE_FD(qmp_fds[0]);
ASSERT_OK(qmp_client_attach_event(client, event, SD_EVENT_PRIORITY_NORMAL));
src/vmspawn/vmspawn-qmp.c
+4 -2
View File
@@ -1863,19 +1863,21 @@ static int probe_schema(QmpClient *c, VmspawnQmpBridge *bridge) {
int vmspawn_qmp_init(VmspawnQmpBridge **ret, int fd, sd_event *event) {
_cleanup_(vmspawn_qmp_bridge_freep) VmspawnQmpBridge *bridge = NULL;
_cleanup_close_ int fd_close = ASSERT_FD(TAKE_FD(fd));
int r;
assert(ret);
assert(fd >= 0);
assert(event);
/* This function always takes ownership of the passed fd. */
bridge = new0(VmspawnQmpBridge, 1);
if (!bridge)
return log_oom();
bridge->scsi_controller_port_idx = -1;
r = qmp_client_connect_fd(&bridge->qmp, fd);
r = qmp_client_connect_fd(&bridge->qmp, TAKE_FD(fd_close));
if (r < 0)
return log_error_errno(r, "Failed to create QMP client: %m");
src/vmspawn/vmspawn.c
+1 -3
View File
@@ -3865,12 +3865,10 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
/* Connect to VMM backend */
_cleanup_(vmspawn_qmp_bridge_freep) VmspawnQmpBridge *bridge = NULL;
r = vmspawn_qmp_init(&bridge, bridge_fds[0], event);
r = vmspawn_qmp_init(&bridge, TAKE_FD(bridge_fds[0]), event);
if (r < 0)
return r;
TAKE_FD(bridge_fds[0]);
/* Probe QEMU feature availability synchronously before device setup consumes the flags. */
r = vmspawn_qmp_probe_features(bridge);
if (r < 0)