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

libsystemd,varlink: always return two fds in varlink upgrade API

Browse Source 
This commit tweaks the API of sd_varlink_call_and_upgrade and
sd_varlink_reply_and_upgrade to return two independent fds even
if the internal {input,output}_fd are the same (e.g. a socket).

This makes the external API easier as there is no longer the risk
of double close. The sd_varlink_call_and_upgrade() is not in a
released version of systemd yet so I presume it is okay to update
it still.

This also allowed some simplifications in varlinkctl.c now that
the handling is easier.
This commit is contained in:
Michael Vogt
2026-04-09 13:02:11 +02:00
committed by Daan De Meyer
parent 34f29079fd
commit 0d21b105d2
4 changed files with 23 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libsystemd/sd-varlink/sd-varlink.c
+10 -12
View File
@@ -2415,29 +2415,27 @@ static int varlink_handle_upgrade_fds(sd_varlink *v, int *ret_input_fd, int *ret
return varlink_log_errno(v, r, "Failed to set output fd to blocking mode: %m");
}
/* Hand out the fds to the caller. When the caller doesn't want one direction, shut it
* down: but avoid closing the underlying fd if the other direction still needs it
* (i.e. when input_fd == output_fd). */
bool same_fd = v->input_fd == v->output_fd;
/* For bidirectional sockets (input_fd == output_fd), dup the fd so that callers
* always get two independent fds they can close separately. */
if (v->input_fd == v->output_fd) {
v->output_fd = fcntl(v->input_fd, F_DUPFD_CLOEXEC, 3);
if (v->output_fd < 0)
return varlink_log_errno(v, errno, "Failed to dup upgraded connection fd: %m");
}
/* Hand out requested fds, shut down unwanted directions. */
if (ret_input_fd)
*ret_input_fd = TAKE_FD(v->input_fd);
else {
(void) shutdown(v->input_fd, SHUT_RD);
if (same_fd && ret_output_fd)
TAKE_FD(v->input_fd); /* don't close yet, output branch needs it */
else
v->input_fd = safe_close(v->input_fd);
v->input_fd = safe_close(v->input_fd);
}
if (ret_output_fd)
*ret_output_fd = TAKE_FD(v->output_fd);
else {
(void) shutdown(v->output_fd, SHUT_WR);
if (same_fd && ret_input_fd)
TAKE_FD(v->output_fd);
else
v->output_fd = safe_close(v->output_fd);
v->output_fd = safe_close(v->output_fd);
}
return 0;
src/systemd/sd-varlink.h
+6 -6
View File
@@ -137,9 +137,9 @@ int sd_varlink_callb(sd_varlink *v, const char *method, sd_json_variant **ret_pa
sd_varlink_callb((v), (method), (ret_parameters), (ret_error_id), SD_JSON_BUILD_OBJECT(__VA_ARGS__))
/* Send method call with upgrade, wait for reply, then steal the connection fds for raw I/O.
* For bidirectional sockets ret_input_fd and ret_output_fd will be the same fd. Callers
* that need independent fds should dup() one of them. ret_parameters and ret_error_id are
* borrowed references valid only until v is closed or unreffed.
* For bidirectional sockets ret_input_fd and ret_output_fd will be separate (dupped) fds
* referring to the same underlying socket. ret_parameters and ret_error_id are borrowed
* references valid only until v is closed or unreffed.
* Returns > 0 if the connection was upgraded, 0 if a Varlink error occurred (and ret_error_id was set),
* or < 0 on local failure. */
int sd_varlink_call_and_upgrade(sd_varlink *v, const char *method, sd_json_variant *parameters, sd_json_variant **ret_parameters, const char **ret_error_id, int *ret_input_fd, int *ret_output_fd);
@@ -171,9 +171,9 @@ int sd_varlink_replyb(sd_varlink *v, ...);
/* Send a final reply to an upgrade request, then steal the connection fds for raw I/O.
* The fds are returned in blocking mode. The varlink connection is disconnected afterwards.
* For bidirectional sockets ret_input_fd and ret_output_fd will be the same fd. Callers
* that need independent fds should dup() one of them. For pipe pairs (e.g. ssh-exec
* transport) they will differ. Either ret pointer may be NULL.
* For bidirectional sockets ret_input_fd and ret_output_fd will be separate (dupped) fds
* referring to the same underlying socket. For pipe pairs (e.g. ssh-exec transport) they
* will differ. Either ret pointer may be NULL.
*
* Note: this call synchronously blocks until the reply is flushed to the socket. This is
* usually fine as flush is fast but a misbehaving/adversary client that stops reading
src/test/test-varlink.c
+5 -14
View File
@@ -763,10 +763,6 @@ static int method_upgrade(sd_varlink *link, sd_json_variant *parameters, sd_varl
if (r < 0)
return r;
/* For a socketpair connection, both fds point to the same socket — avoid double-close */
if (input_fd == output_fd)
output_fd = -EBADF;
/* After upgrade, do raw I/O: read until EOF, reverse, write back.
* The client shuts down its write side after sending, so we get a clean EOF. */
char buf[64] = {};
@@ -777,8 +773,7 @@ static int method_upgrade(sd_varlink *link, sd_json_variant *parameters, sd_varl
for (ssize_t i = 0; i < n / 2; i++)
SWAP_TWO(buf[i], buf[n - 1 - i]);
int write_fd = output_fd >= 0 ? output_fd : input_fd;
ASSERT_OK(loop_write(write_fd, buf, n));
ASSERT_OK(loop_write(output_fd, buf, n));
return 0;
}
@@ -807,16 +802,12 @@ static void *upgrade_thread(void *arg) {
ASSERT_NULL(error_id);
ASSERT_GE(input_fd, 0);
ASSERT_GE(output_fd, 0);
ASSERT_NE(input_fd, output_fd); /* library dups for bidirectional sockets */
/* For a socketpair connection, both fds point to the same socket — avoid double-close */
if (input_fd == output_fd)
output_fd = -EBADF;
/* Send a test string, expect reversed reply */
/* Send a test string, shut down write side so server sees EOF, then read the reversed reply */
static const char msg[] = "Hello!";
int write_fd = output_fd >= 0 ? output_fd : input_fd;
ASSERT_OK(loop_write(write_fd, msg, strlen(msg)));
ASSERT_OK_ERRNO(shutdown(write_fd, SHUT_WR));
ASSERT_OK(loop_write(output_fd, msg, strlen(msg)));
ASSERT_OK_ERRNO(shutdown(output_fd, SHUT_WR));
char buf[64] = {};
ssize_t n = ASSERT_OK(loop_read(input_fd, buf, strlen(msg), /* do_poll= */ true));
src/varlinkctl/varlinkctl.c
+2 -14
View File
@@ -672,15 +672,6 @@ static int varlink_call_and_upgrade(const char *url, const char *method, sd_json
if (!isempty(error_id))
return log_error_errno(SYNTHETIC_ERRNO(EBADE), "Upgrade via %s() failed with error: %s", method, error_id);
/* For bidirectional sockets input_fd == output_fd. Dup immediately so that _cleanup_close_
* on both variables can never double-close the same fd. Note that on fcntl() failure
* output_fd is overwritten with -1, so only input_fd holds the real fd at cleanup time. */
if (input_fd == output_fd) {
output_fd = fcntl(input_fd, F_DUPFD_CLOEXEC, 3);
if (output_fd < 0)
return log_error_errno(errno, "Failed to dup upgraded connection fd: %m");
}
if (!strv_isempty(exec_cmdline)) {
/* --exec mode: place the upgraded connection on stdin/stdout so that the child
* process can just read/write naturally. */
@@ -1213,8 +1204,8 @@ static int varlink_server_add_interface_from_method(sd_varlink_server *s, const
static int method_serve_upgrade(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) {
char **exec_cmdline = ASSERT_PTR(userdata);
_cleanup_close_ int input_fd = -EBADF, _output_fd = -EBADF;
int output_fd, r;
_cleanup_close_ int input_fd = -EBADF, output_fd = -EBADF;
int r;
if (!FLAGS_SET(flags, SD_VARLINK_METHOD_UPGRADE))
return sd_varlink_error(link, SD_VARLINK_ERROR_EXPECTED_UPGRADE, NULL);
@@ -1223,9 +1214,6 @@ static int method_serve_upgrade(sd_varlink *link, sd_json_variant *parameters, s
if (r < 0)
return log_error_errno(r, "Failed to upgrade connection: %m");
if (output_fd != input_fd)
_output_fd = output_fd;
/* Copy exec_cmdline before forking: pidref_safe_fork() calls rename_process() which
* overwrites the argv area that exec_cmdline points into. */
_cleanup_strv_free_ char **cmdline_copy = strv_copy(exec_cmdline);