Skip to content

Commit b4a2de9

Browse files
DemiMarieDemiMarie
committed
Don't close file descriptor 0
Also save some FD manipulation during process_io().
1 parent f1370c5 commit b4a2de9

File tree

2 files changed

+52
-40
lines changed

2 files changed

+52
-40
lines changed

‎daemon/qrexec-daemon.c‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,6 @@ static void init(int xid)
327327
}
328328
}
329329

330-
close(0);
331330

332331
if (!opt_direct) {
333332
snprintf(qrexec_error_log_name, sizeof(qrexec_error_log_name),
@@ -625,25 +624,26 @@ static void handle_cmdline_message_from_client(int fd)
625624

626625
static void handle_client_hello(int fd)
627626
{
628-
struct msg_header hdr;
629-
struct peer_info info;
630-
631-
if (!read_all(fd, &hdr, sizeof hdr)) {
627+
struct {
628+
struct msg_header hdr;
629+
struct peer_info info;
630+
} data;
631+
_Static_assert(sizeof data == sizeof(data.hdr) + sizeof(data.info),
632+
"unexpected padding");
633+
634+
if (!read_all(fd, &data, sizeof data)) {
632635
terminate_client(fd);
633636
return;
634637
}
635-
if (hdr.type != MSG_HELLO || hdr.len != sizeof(info)) {
638+
if (data.hdr.type != MSG_HELLO || data.hdr.len != sizeof(data.info)) {
636639
LOG(ERROR, "Invalid HELLO packet received from client %d: "
637-
"type %d, len %d", fd, hdr.type, hdr.len);
640+
"type %d, len %d", fd, data.hdr.type, data.hdr.len);
638641
terminate_client(fd);
639642
return;
640643
}
641-
if (!read_all(fd, &info, sizeof info)) {
642-
terminate_client(fd);
643-
return;
644-
}
645-
if (info.version != QREXEC_PROTOCOL_VERSION) {
646-
LOG(ERROR, "Incompatible client protocol version (remote %d, local %d)", info.version, QREXEC_PROTOCOL_VERSION);
644+
if (data.info.version != QREXEC_PROTOCOL_VERSION) {
645+
LOG(ERROR, "Incompatible client protocol version (remote %d, local %d)",
646+
data.info.version, QREXEC_PROTOCOL_VERSION);
647647
terminate_client(fd);
648648
return;
649649
}
@@ -1484,6 +1484,18 @@ int main(int argc, char **argv)
14841484
int i, opt;
14851485
sigset_t selectmask;
14861486

1487+
{
1488+
int null_fd = open("/dev/null", O_RDONLY|O_NOCTTY);
1489+
if (null_fd < 0)
1490+
err(1, "open(%s)", "/dev/null");
1491+
if (null_fd > 0) {
1492+
if (dup2(null_fd, 0) != 0)
1493+
err(1, "dup2(%d, 0)", null_fd);
1494+
if (null_fd > 2 && close(null_fd) != 0)
1495+
err(1, "close(%d)", null_fd);
1496+
}
1497+
}
1498+
14871499
setup_logging("qrexec-daemon");
14881500

14891501
while ((opt=getopt_long(argc, argv, "hqp:D", longopts, NULL)) != -1) {

‎libqrexec/process_io.c‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,36 +47,40 @@ static _Noreturn void handle_vchan_error(const char *op)
4747
* don't care, because we created it)
4848
*/
4949

50-
static void close_stdin(int fd, bool restore_block) {
50+
static void close_stdin(int fd, bool restore_block, bool close_fd) {
5151
if (fd == -1)
5252
return;
5353

5454
if (restore_block)
5555
set_block(fd);
5656

5757
if (fd == 1) {
58-
close(fd);
59-
} else if (shutdown(fd, SHUT_WR) == -1) {
60-
if (errno == ENOTSOCK)
58+
if (close_fd)
6159
close(fd);
62-
else
60+
} else if (shutdown(fd, SHUT_WR) == -1) {
61+
if (errno == ENOTSOCK) {
62+
if (close_fd)
63+
close(fd);
64+
} else
6365
PERROR("shutdown close_stdin");
6466
}
6567
}
6668

67-
static void close_stdout(int fd, bool restore_block) {
69+
static void close_stdout(int fd, bool restore_block, bool close_fd) {
6870
if (fd == -1)
6971
return;
7072

7173
if (restore_block)
7274
set_block(fd);
7375

7476
if (fd == 0) {
75-
close(fd);
76-
} else if (shutdown(fd, SHUT_RD) == -1) {
77-
if (errno == ENOTSOCK)
77+
if (close_fd)
7878
close(fd);
79-
else
79+
} else if (shutdown(fd, SHUT_RD) == -1) {
80+
if (errno == ENOTSOCK) {
81+
if (close_fd)
82+
close(fd);
83+
} else
8084
PERROR("shutdown close_stdout");
8185
}
8286
}
@@ -117,7 +121,7 @@ int process_io(const struct process_io_request *req) {
117121
pid_t local_status = -1;
118122
pid_t remote_status = -1;
119123
int stdout_msg_type = is_service ? MSG_DATA_STDOUT : MSG_DATA_STDIN;
120-
bool use_stdio_socket = false;
124+
bool use_stdio_socket = stdin_fd == stdout_fd;
121125

122126
int ret;
123127
struct pollfd fds[FD_NUM];
@@ -134,8 +138,6 @@ int process_io(const struct process_io_request *req) {
134138
set_nonblock(stdin_fd);
135139
if (stdout_fd != stdin_fd)
136140
set_nonblock(stdout_fd);
137-
else if ((stdout_fd = fcntl(stdin_fd, F_DUPFD_CLOEXEC, 3)) < 0)
138-
abort(); // not worth handling running out of file descriptors
139141
if (stderr_fd >= 0)
140142
set_nonblock(stderr_fd);
141143

@@ -149,7 +151,7 @@ int process_io(const struct process_io_request *req) {
149151
else
150152
local_status = WEXITSTATUS(status);
151153
if (stdin_fd >= 0) {
152-
close_stdin(stdin_fd, !use_stdio_socket);
154+
close_stdin(stdin_fd, !use_stdio_socket, stdin_fd != stdout_fd);
153155
stdin_fd = -1;
154156
}
155157
}
@@ -194,14 +196,12 @@ int process_io(const struct process_io_request *req) {
194196
}
195197

196198
/* child signaled desire to use single socket for both stdin and stdout */
197-
if (sigusr1 && *sigusr1) {
199+
if (sigusr1 && *sigusr1 && stdin_fd != stdout_fd) {
198200
if (stdout_fd != -1) {
199-
do
200-
errno = 0;
201-
while (dup3(stdin_fd, stdout_fd, O_CLOEXEC) &&
202-
(errno == EINTR || errno == EBUSY));
203-
// other errors are fatal
204-
if (errno) {
201+
while (dup3(stdin_fd, stdout_fd, O_CLOEXEC) == -1) {
202+
if (errno == EINTR || errno == EBUSY)
203+
continue;
204+
// other errors are fatal
205205
PERROR("dup3");
206206
abort();
207207
}
@@ -265,7 +265,7 @@ int process_io(const struct process_io_request *req) {
265265
handle_vchan_error("wait");
266266

267267
if (stdin_fd >= 0 && fds[FD_STDIN].revents & (POLLHUP | POLLERR)) {
268-
close_stdin(stdin_fd, !use_stdio_socket);
268+
close_stdin(stdin_fd, !use_stdio_socket, stdin_fd != stdout_fd);
269269
stdin_fd = -1;
270270
}
271271

@@ -282,15 +282,15 @@ int process_io(const struct process_io_request *req) {
282282
handle_vchan_error("read");
283283
break;
284284
case REMOTE_EOF:
285-
close_stdin(stdin_fd, !use_stdio_socket);
285+
close_stdin(stdin_fd, !use_stdio_socket, stdin_fd != stdout_fd);
286286
stdin_fd = -1;
287287
break;
288288
case REMOTE_EXITED:
289289
/* Remote process exited, we don't need any more data from
290290
* local FDs. However, don't exit yet, because there might
291291
* still be some data in stdin_buf waiting to be flushed.
292292
*/
293-
close_stdout(stdout_fd, !use_stdio_socket);
293+
close_stdout(stdout_fd, !use_stdio_socket, stdin_fd != stdout_fd);
294294
stdout_fd = -1;
295295
close_stderr(stderr_fd);
296296
stderr_fd = -1;
@@ -304,7 +304,7 @@ int process_io(const struct process_io_request *req) {
304304
handle_vchan_error("send(handle_input stdout)");
305305
break;
306306
case REMOTE_EOF:
307-
close_stdout(stdout_fd, !use_stdio_socket);
307+
close_stdout(stdout_fd, !use_stdio_socket, stdin_fd != stdout_fd);
308308
stdout_fd = -1;
309309
break;
310310
}
@@ -325,8 +325,8 @@ int process_io(const struct process_io_request *req) {
325325
}
326326
/* make sure that all the pipes/sockets are closed, so the child process
327327
* (if any) will know that the connection is terminated */
328-
close_stdin(stdin_fd, true);
329-
close_stdout(stdout_fd, true);
328+
close_stdin(stdin_fd, true, stdin_fd != stdout_fd);
329+
close_stdout(stdout_fd, true, true);
330330
close_stderr(stderr_fd);
331331

332332
/* wait for local process, in case we exited early */

0 commit comments

Comments
 (0)