Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/pr/143'
Browse files Browse the repository at this point in the history 
* origin/pr/143:
  Don't close file descriptor 0
  Use libvchan_client_init_async() instead of parent process timeout
  Avoid using signal() to establish a signal handler
  Use SOCK_CLOEXEC instead of setting O_CLOEXEC manually
  Use a pipe instead of signals to notify readiness
  Use sigemptyset() to initialize signal sets
  Avoid using alarm(2) for timeouts
  Add exit codes to qrexec.h
  • Loading branch information
@marmarek
marmarek committed Apr 18, 2024
2 parents b0c56f1 + a5de843 commit 9401273
Show file tree
Hide file tree
Showing 15 changed files with 210 additions and 189 deletions.
78 changes: 11 additions & 67 deletions agent/qrexec-agent-data.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,14 @@ static void sigusr1_handler(int __attribute__((__unused__))x)
void prepare_child_env(void) {
char pid_s[10];

signal(SIGCHLD, sigchld_handler);
signal(SIGUSR1, sigusr1_handler);
struct sigaction action = {
.sa_handler = sigchld_handler,
.sa_flags = 0,
};
sigemptyset(&action.sa_mask);
if (sigaction(SIGCHLD, &action, NULL)) abort();
action.sa_handler = sigusr1_handler;
if (sigaction(SIGUSR1, &action, NULL)) abort();
int res = snprintf(pid_s, sizeof(pid_s), "%d", getpid());
if (res < 0) abort();
if (res >= (int)sizeof(pid_s)) abort();
Expand Down Expand Up @@ -160,69 +166,6 @@ static int handle_just_exec(struct qrexec_parsed_command *cmd)
return 0;
}

static const long BILLION_NANOSECONDS = 1000000000L;

static int wait_for_vchan_connection_with_timeout(
libvchan_t *conn, int wait_fd, bool is_server, time_t timeout) {
struct timespec end_tp, now_tp, timeout_tp;

if (timeout && clock_gettime(CLOCK_MONOTONIC, &end_tp)) {
PERROR("clock_gettime");
return -1;
}
assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS);
end_tp.tv_sec += timeout;
while (true) {
bool did_timeout = true;
struct pollfd fds = { .fd = wait_fd, .events = POLLIN | POLLHUP, .revents = 0 };

/* calculate how much time left until connection timeout expire */
if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) {
PERROR("clock_gettime");
return -1;
}
assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS);
if (now_tp.tv_sec <= end_tp.tv_sec) {
timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec;
timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec;
if (timeout_tp.tv_nsec < 0) {
timeout_tp.tv_nsec += BILLION_NANOSECONDS;
timeout_tp.tv_sec--;
}
did_timeout = timeout_tp.tv_sec < 0;
}
switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) {
case -1:
if (errno == EINTR)
break;
LOG(ERROR, "vchan connection error");
return -1;
case 0:
LOG(ERROR, "vchan connection timeout");
return -1;
case 1:
break;
default:
abort();
}
if (fds.revents & POLLIN) {
if (is_server) {
libvchan_wait(conn);
return 0;
} else {
int connect_ret = libvchan_client_init_async_finish(conn, true);

if (connect_ret < 0) {
LOG(ERROR, "vchan connection error");
return -1;
} else if (connect_ret == 0) {
return 0;
}
}
}
}
}


/* Behaviour depends on type parameter:
* MSG_JUST_EXEC - connect to vchan server, fork+exec process given by cmdline
Expand Down Expand Up @@ -263,7 +206,8 @@ static int handle_new_process_common(
LOG(ERROR, "Data vchan connection failed");
exit(1);
}
if (wait_for_vchan_connection_with_timeout(data_vchan, wait_fd, false, connection_timeout) < 0) {
if (qubes_wait_for_vchan_connection_with_timeout(
data_vchan, wait_fd, false, connection_timeout) < 0) {
LOG(ERROR, "Data vchan connection failed");
exit(1);
}
Expand Down Expand Up @@ -391,7 +335,7 @@ int handle_data_client(
LOG(ERROR, "Data vchan connection failed");
exit(1);
}
if (wait_for_vchan_connection_with_timeout(
if (qubes_wait_for_vchan_connection_with_timeout(
data_vchan, libvchan_fd_for_select(data_vchan), true, connection_timeout) < 0) {
LOG(ERROR, "Data vchan connection failed");
exit(1);
Expand Down
10 changes: 8 additions & 2 deletions agent/qrexec-agent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -928,8 +928,14 @@ int main(int argc, char **argv)
}

init();
signal(SIGCHLD, sigchld_handler);
signal(SIGTERM, sigterm_handler);
struct sigaction action = {
.sa_handler = sigchld_handler,
.sa_flags = SA_RESTART,
};
sigemptyset(&action.sa_mask);
sigaction(SIGCHLD, &action, NULL);
action.sa_handler = sigterm_handler;
sigaction(SIGTERM, &action, NULL);
signal(SIGPIPE, SIG_IGN);

sigemptyset(&selectmask);
Expand Down
4 changes: 0 additions & 4 deletions agent/qrexec-fork-server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,6 @@ int main(int argc, char **argv) {
}

s = get_server_socket(socket_path);
if (fcntl(s, F_SETFD, O_CLOEXEC) < 0) {
PERROR("fcntl");
exit(1);
}
/* fork into background */
switch (fork()) {
case -1:
Expand Down
62 changes: 9 additions & 53 deletions daemon/qrexec-client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,57 +143,6 @@ static void parse_connect(char *str, char **request_id,
exit(1);
}

static const long BILLION_NANOSECONDS = 1000000000L;

static void wait_for_vchan_client_with_timeout(libvchan_t *conn, time_t timeout) {
struct timespec end_tp, now_tp, timeout_tp;

if (timeout && clock_gettime(CLOCK_MONOTONIC, &end_tp)) {
PERROR("clock_gettime");
exit(1);
}
assert(end_tp.tv_nsec >= 0 && end_tp.tv_nsec < BILLION_NANOSECONDS);
end_tp.tv_sec += timeout;
int const fd = libvchan_fd_for_select(conn);
while (conn && libvchan_is_open(conn) == VCHAN_WAITING) {
if (timeout) {
bool did_timeout = true;
struct pollfd fds = { .fd = fd, .events = POLLIN | POLLHUP, .revents = 0 };

/* calculate how much time left until connection timeout expire */
if (clock_gettime(CLOCK_MONOTONIC, &now_tp)) {
PERROR("clock_gettime");
exit(1);
}
assert(now_tp.tv_nsec >= 0 && now_tp.tv_nsec < BILLION_NANOSECONDS);
if (now_tp.tv_sec <= end_tp.tv_sec) {
timeout_tp.tv_sec = end_tp.tv_sec - now_tp.tv_sec;
timeout_tp.tv_nsec = end_tp.tv_nsec - now_tp.tv_nsec;
if (timeout_tp.tv_nsec < 0) {
timeout_tp.tv_nsec += BILLION_NANOSECONDS;
timeout_tp.tv_sec--;
}
did_timeout = timeout_tp.tv_sec < 0;
}
switch (did_timeout ? 0 : ppoll(&fds, 1, &timeout_tp, NULL)) {
case -1:
if (errno == EINTR)
break;
LOG(ERROR, "vchan connection error");
exit(1);
case 0:
LOG(ERROR, "vchan connection timeout");
exit(1);
case 1:
break;
default:
abort();
}
}
libvchan_wait(conn);
}
}

static size_t compute_service_length(const char *const remote_cmdline, const char *const prog_name) {
const size_t service_length = strlen(remote_cmdline) + 1;
if (service_length < 2 || service_length > MAX_QREXEC_CMD_LEN) {
Expand Down Expand Up @@ -365,9 +314,16 @@ int main(int argc, char **argv)
VCHAN_BUFFER_SIZE, VCHAN_BUFFER_SIZE);
if (!data_vchan) {
LOG(ERROR, "Failed to start data vchan server");
exit(1);
rc = QREXEC_EXIT_PROBLEM;
goto cleanup;
}
int fd = libvchan_fd_for_select(data_vchan);
if (qubes_wait_for_vchan_connection_with_timeout(
data_vchan, fd, true, connection_timeout) < 0) {
LOG(ERROR, "qrexec connection timeout");
rc = QREXEC_EXIT_PROBLEM;
goto cleanup;
}
wait_for_vchan_client_with_timeout(data_vchan, connection_timeout);
struct handshake_params params = {
.data_vchan = data_vchan,
.stdin_buffer = &stdin_buffer,
Expand Down
35 changes: 18 additions & 17 deletions daemon/qrexec-daemon-common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ int handle_agent_handshake(libvchan_t *vchan, bool remote_send_first)
static void sigchld_handler(int x __attribute__((__unused__)))
{
sigchld = 1;
signal(SIGCHLD, sigchld_handler);
}

/* See also qrexec-agent.c:wait_for_session_maybe() */
Expand Down Expand Up @@ -272,7 +271,12 @@ int prepare_local_fds(struct qrexec_parsed_command *command, struct buffer *stdi
{
if (stdin_buffer == NULL)
abort();
if (signal(SIGCHLD, sigchld_handler) == SIG_ERR)
struct sigaction action = {
.sa_handler = sigchld_handler,
.sa_flags = 0,
};
sigemptyset(&action.sa_mask);
if (sigaction(SIGCHLD, &action, NULL))
return 126;
return execute_parsed_qubes_rpc_command(command, &local_pid, &local_stdin_fd, &local_stdout_fd,
NULL, stdin_buffer);
Expand Down Expand Up @@ -331,12 +335,6 @@ static int select_loop(struct handshake_params *params)
return (params->exit_with_code ? exit_code : 0);
}

static void sigalrm_handler(int x __attribute__((__unused__)))
{
LOG(ERROR, "vchan connection timeout");
_exit(1);
}

int run_qrexec_to_dom0(const struct service_params *svc_params,
int src_domain_id,
const char *src_domain_name,
Expand Down Expand Up @@ -375,15 +373,18 @@ int run_qrexec_to_dom0(const struct service_params *svc_params,
} else {
prepare_ret = prepare_local_fds(command, &stdin_buffer);
}
void (*old_handler)(int);

/* libvchan_client_init is blocking and does not support connection
* timeout, so use alarm(2) for that... */
old_handler = signal(SIGALRM, sigalrm_handler);
alarm(connection_timeout);
data_vchan = libvchan_client_init(data_domain, data_port);
alarm(0);
signal(SIGALRM, old_handler);
int wait_fd;
data_vchan = libvchan_client_init_async(data_domain, data_port, &wait_fd);
if (!data_vchan) {
LOG(ERROR, "Cannot create data vchan connection");
return QREXEC_EXIT_PROBLEM;
}
if (qubes_wait_for_vchan_connection_with_timeout(
data_vchan, wait_fd, false, connection_timeout) < 0) {
LOG(ERROR, "qrexec connection timeout");
return QREXEC_EXIT_PROBLEM;
}

struct handshake_params params = {
.data_vchan = data_vchan,
.stdin_buffer = &stdin_buffer,
Expand Down
Loading

0 comments on commit 9401273

Please sign in to comment.