December 2018 - tor-commits - lists.torproject.org

[tor/master] Add missing changes files for #26360, #28179, #28180.
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit 44a80bb36115ebf865ba688a0f0d0f8023d992d6 Author: Alexander Færøy <ahf(a)torproject.org> Date: Mon Dec 17 23:41:53 2018 +0100 Add missing changes files for #26360, #28179, #28180. See: https://bugs.torproject.org/26179 See: https://bugs.torproject.org/28180 See: https://bugs.torproject.org/28360 --- changes/ticket26360 | 4 ++++ changes/ticket28179 | 5 +++++ changes/ticket28180 | 3 +++ 3 files changed, 12 insertions(+) diff --git a/changes/ticket26360 b/changes/ticket26360 new file mode 100644 index 000000000..80afbd1c1 --- /dev/null +++ b/changes/ticket26360 @@ -0,0 +1,4 @@ + o Minor bugfixes (pluggable transports): + - Make sure that data is continously read from standard out and error of the + PT child-process to avoid deadlocking when the pipes' buffer is full. + Fixes bug 26360; bugfix on 0.2.3.6-alpha. diff --git a/changes/ticket28179 b/changes/ticket28179 new file mode 100644 index 000000000..f548c4a79 --- /dev/null +++ b/changes/ticket28179 @@ -0,0 +1,5 @@ + o Minor features (process): + - Add new Process API for handling child processes. This + new API allows Tor to have bi-directional + communication with child processes on both Unix and Windows. + Closes ticket 28179. diff --git a/changes/ticket28180 b/changes/ticket28180 new file mode 100644 index 000000000..2ec547bd4 --- /dev/null +++ b/changes/ticket28180 @@ -0,0 +1,3 @@ + o Minor features (pluggable transports): + - Add support for logging to Tor's logging subsystem from a pluggable + transport process. Partial implementation for ticket 28180

1 0

[tor/master] Avoid breaking the event loop prematurely.
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit ca7a2ecc512145589964577f2f2ff6133ab146f5 Author: Alexander Færøy <ahf(a)torproject.org> Date: Tue Dec 18 15:18:22 2018 +0100 Avoid breaking the event loop prematurely. This patch makes sure that we terminate the event loop from the event loop timer instead of directly in the process' exit handler. This allows us to run the event loop an additional time to ensure that the SleepEx() call on Windows is called and the data from stdout/stderr is delivered to us. Additionally we ensure that we don't try to read or write data from a Unix process that have been terminated in the main loop, since its file descriptors are closed at that time. See: https://bugs.torproject.org/28179 --- src/lib/process/process_unix.c | 21 +++++++++++---------- src/test/test_process_slow.c | 42 ++++++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/lib/process/process_unix.c b/src/lib/process/process_unix.c index 4fc1b7a51..57ca69a76 100644 --- a/src/lib/process/process_unix.c +++ b/src/lib/process/process_unix.c @@ -112,16 +112,6 @@ process_unix_free_(process_unix_t *unix_process) /* FIXME(ahf): Refactor waitpid code? */ unix_process->waitpid = NULL; - /* Cleanup our events. */ - if (! unix_process->stdout_handle.reached_eof) - process_unix_stop_reading(&unix_process->stdout_handle); - - if (! unix_process->stderr_handle.reached_eof) - process_unix_stop_reading(&unix_process->stderr_handle); - - if (unix_process->stdin_handle.is_writing) - process_unix_stop_writing(&unix_process->stdin_handle); - /* Close all our file descriptors. */ process_unix_close_file_descriptors(unix_process); @@ -668,6 +658,17 @@ process_unix_close_file_descriptors(process_unix_t *unix_process) int ret; bool success = true; + /* Stop reading and writing before we close() our + * file descriptors. */ + if (! unix_process->stdout_handle.reached_eof) + process_unix_stop_reading(&unix_process->stdout_handle); + + if (! unix_process->stderr_handle.reached_eof) + process_unix_stop_reading(&unix_process->stderr_handle); + + if (unix_process->stdin_handle.is_writing) + process_unix_stop_writing(&unix_process->stdin_handle); + if (unix_process->stdin_handle.fd != -1) { ret = close(unix_process->stdin_handle.fd); if (ret == -1) { diff --git a/src/test/test_process_slow.c b/src/test/test_process_slow.c index cc7e8c5a9..a1f99bff0 100644 --- a/src/test/test_process_slow.c +++ b/src/test/test_process_slow.c @@ -36,6 +36,7 @@ struct process_data_t { smartlist_t *stderr_data; smartlist_t *stdin_data; process_exit_code_t exit_code; + bool did_exit; }; typedef struct process_data_t process_data_t; @@ -97,13 +98,17 @@ process_stderr_callback(process_t *process, const char *data, size_t size) static bool process_exit_callback(process_t *process, process_exit_code_t exit_code) { + process_status_t status; + tt_ptr_op(process, OP_NE, NULL); process_data_t *process_data = process_get_data(process); process_data->exit_code = exit_code; + process_data->did_exit = true; - /* Our process died. Let's check the values it returned. */ - tor_shutdown_event_loop_and_exit(0); + /* Check if our process is still running? */ + status = process_get_status(process); + tt_int_op(status, OP_EQ, PROCESS_STATUS_NOT_RUNNING); done: /* Do not free up our process_t. */ @@ -137,9 +142,16 @@ main_loop_timeout_cb(periodic_timer_t *timer, void *data) { /* Sanity check. */ tt_ptr_op(timer, OP_EQ, main_loop_timeout_timer); - tt_ptr_op(data, OP_EQ, NULL); + tt_ptr_op(data, OP_NE, NULL); + + /* Our process data. */ + process_data_t *process_data = data; + + /* Our process did exit. */ + if (process_data->did_exit) + tor_shutdown_event_loop_and_exit(0); - /* Have we been called 10 times we exit. */ + /* Have we been called 10 times we exit the main loop. */ timer_tick_count++; tt_int_op(timer_tick_count, OP_LT, 10); @@ -156,7 +168,7 @@ main_loop_timeout_cb(periodic_timer_t *timer, void *data) } static void -run_main_loop(void) +run_main_loop(process_data_t *process_data) { int ret; @@ -167,7 +179,7 @@ run_main_loop(void) main_loop_timeout_timer = periodic_timer_new(tor_libevent_get_base(), &interval, main_loop_timeout_cb, - NULL); + process_data); /* Run our main loop. */ ret = run_main_loop_until_done(); @@ -225,11 +237,7 @@ test_callbacks(void *arg) process_printf(process, " lines?\r\n"); /* Start our main loop. */ - run_main_loop(); - - /* Check if our process is still running? */ - status = process_get_status(process); - tt_int_op(status, OP_EQ, PROCESS_STATUS_NOT_RUNNING); + run_main_loop(process_data); /* We returned. Let's see what our event loop said. */ tt_int_op(smartlist_len(process_data->stdout_data), OP_EQ, 12); @@ -310,14 +318,16 @@ test_callbacks_terminate(void *arg) tt_int_op(status, OP_EQ, PROCESS_STATUS_RUNNING); /* Zap our process. */ - process_terminate(process); + bool success; + + success = process_terminate(process); + tt_assert(success); /* Start our main loop. */ - run_main_loop(); + run_main_loop(process_data); - /* Check if our process is still running? */ - status = process_get_status(process); - tt_int_op(status, OP_EQ, PROCESS_STATUS_NOT_RUNNING); + /* Check if we did exit. */ + tt_assert(process_data->did_exit); done: process_data_free(process_data);

1 0

[tor/master] Update location of buffers.h
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit 4ad59bfbc274ff0edfe56ff78df0eeba0841415f Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Dec 17 17:01:50 2018 -0500 Update location of buffers.h --- src/lib/process/process.c | 2 +- src/lib/process/process_unix.c | 2 +- src/lib/process/process_win32.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/process/process.c b/src/lib/process/process.c index b3370e919..ae345ceea 100644 --- a/src/lib/process/process.c +++ b/src/lib/process/process.c @@ -9,7 +9,7 @@ **/ #define PROCESS_PRIVATE -#include "lib/container/buffers.h" +#include "lib/buf/buffers.h" #include "lib/net/buffers_net.h" #include "lib/container/smartlist.h" #include "lib/log/log.h" diff --git a/src/lib/process/process_unix.c b/src/lib/process/process_unix.c index 04d6381aa..4fc1b7a51 100644 --- a/src/lib/process/process_unix.c +++ b/src/lib/process/process_unix.c @@ -10,7 +10,7 @@ #define PROCESS_UNIX_PRIVATE #include "lib/intmath/cmp.h" -#include "lib/container/buffers.h" +#include "lib/buf/buffers.h" #include "lib/net/buffers_net.h" #include "lib/container/smartlist.h" #include "lib/evloop/compat_libevent.h" diff --git a/src/lib/process/process_win32.c b/src/lib/process/process_win32.c index e9367dafc..c92b0975a 100644 --- a/src/lib/process/process_win32.c +++ b/src/lib/process/process_win32.c @@ -10,7 +10,7 @@ #define PROCESS_WIN32_PRIVATE #include "lib/intmath/cmp.h" -#include "lib/container/buffers.h" +#include "lib/buf/buffers.h" #include "lib/net/buffers_net.h" #include "lib/container/smartlist.h" #include "lib/log/log.h"

1 0

[tor/master] lib/process may include lib/buf.
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit 8a01f0eaabe8d5975b12328a745d2a6e921ef868 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Dec 17 17:58:49 2018 -0500 lib/process may include lib/buf. --- src/lib/process/.may_include | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/process/.may_include b/src/lib/process/.may_include index 2e90f4d8d..ce1b6ecf5 100644 --- a/src/lib/process/.may_include +++ b/src/lib/process/.may_include @@ -1,5 +1,6 @@ orconfig.h +lib/buf/*.h lib/cc/*.h lib/container/*.h lib/ctime/*.h

1 0

[tor/master] Merge branch 'ticket28179_squashed_merged'
by nickm＠torproject.org 18 Dec '18

18 Dec '18

1 0

[tor/master] Refactor flush_chunk() to work on pipes as well as sockets.
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit 340260281a514ee92b4c40ee5ddf8728ac580a40 Author: Alexander Færøy <ahf(a)torproject.org> Date: Mon Sep 10 13:28:01 2018 +0200 Refactor flush_chunk() to work on pipes as well as sockets. See: https://bugs.torproject.org/28179 --- src/lib/net/buffers_net.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lib/net/buffers_net.c b/src/lib/net/buffers_net.c index bd420510a..fc133a01f 100644 --- a/src/lib/net/buffers_net.c +++ b/src/lib/net/buffers_net.c @@ -129,22 +129,26 @@ buf_read_from_socket(buf_t *buf, tor_socket_t s, size_t at_most, } /** Helper for buf_flush_to_socket(): try to write sz bytes from chunk - * chunk of buffer buf onto socket s. On success, deduct - * the bytes written from *buf_flushlen. Return the number of bytes - * written on success, 0 on blocking, -1 on failure. + * chunk of buffer buf onto file descriptor fd. On + * success, deduct the bytes written from *buf_flushlen. Return the + * number of bytes written on success, 0 on blocking, -1 on failure. */ static inline int -flush_chunk(tor_socket_t s, buf_t *buf, chunk_t *chunk, size_t sz, - size_t *buf_flushlen) +flush_chunk(tor_socket_t fd, buf_t *buf, chunk_t *chunk, size_t sz, + size_t *buf_flushlen, bool is_socket) { ssize_t write_result; if (sz > chunk->datalen) sz = chunk->datalen; - write_result = tor_socket_send(s, chunk->data, sz, 0); + + if (is_socket) + write_result = tor_socket_send(fd, chunk->data, sz, 0); + else + write_result = write(fd, chunk->data, sz); if (write_result < 0) { - int e = tor_socket_errno(s); + int e = tor_socket_errno(fd); if (!ERRNO_IS_EAGAIN(e)) { /* it's a real error */ #ifdef _WIN32 if (e == WSAENOBUFS) @@ -195,7 +199,7 @@ buf_flush_to_socket(buf_t *buf, tor_socket_t s, size_t sz, else flushlen0 = buf->head->datalen; - r = flush_chunk(s, buf, buf->head, flushlen0, buf_flushlen); + r = flush_chunk(s, buf, buf->head, flushlen0, buf_flushlen, true); check(); if (r < 0) return r;

1 0

[tor/master] Refactor read_to_chunk() such that it supports both pipes and sockets.
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit 5f26ae833eea79e56cb8cb8133b16a9cb0944ae4 Author: Alexander Færøy <ahf(a)torproject.org> Date: Mon Sep 10 13:23:59 2018 +0200 Refactor read_to_chunk() such that it supports both pipes and sockets. See: https://bugs.torproject.org/28179 --- src/lib/net/buffers_net.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/lib/net/buffers_net.c b/src/lib/net/buffers_net.c index c52ea2784..bd420510a 100644 --- a/src/lib/net/buffers_net.c +++ b/src/lib/net/buffers_net.c @@ -21,6 +21,7 @@ #endif #include <stdlib.h> +#include <unistd.h> #ifdef PARANOIA /** Helper: If PARANOIA is defined, assert that the buffer in local variable @@ -30,27 +31,33 @@ #define check() STMT_NIL #endif /* defined(PARANOIA) */ -/** Read up to at_most bytes from the socket fd into +/** Read up to at_most bytes from the file descriptor fd into * chunk (which must be on buf). If we get an EOF, set * *reached_eof to 1. Return -1 on error, 0 on eof or blocking, * and the number of bytes read otherwise. */ static inline int read_to_chunk(buf_t *buf, chunk_t *chunk, tor_socket_t fd, size_t at_most, - int *reached_eof, int *socket_error) + int *reached_eof, int *error, bool is_socket) { ssize_t read_result; if (at_most > CHUNK_REMAINING_CAPACITY(chunk)) at_most = CHUNK_REMAINING_CAPACITY(chunk); - read_result = tor_socket_recv(fd, CHUNK_WRITE_PTR(chunk), at_most, 0); + + if (is_socket) + read_result = tor_socket_recv(fd, CHUNK_WRITE_PTR(chunk), at_most, 0); + else + read_result = read(fd, CHUNK_WRITE_PTR(chunk), at_most); if (read_result < 0) { int e = tor_socket_errno(fd); if (!ERRNO_IS_EAGAIN(e)) { /* it's a real error */ #ifdef _WIN32 if (e == WSAENOBUFS) - log_warn(LD_NET,"recv() failed: WSAENOBUFS. Not enough ram?"); + log_warn(LD_NET, "%s() failed: WSAENOBUFS. Not enough ram?", + is_socket ? "recv" : "read"); #endif - *socket_error = e; + if (error) + *error = e; return -1; } return 0; /* would block. */ @@ -108,7 +115,7 @@ buf_read_from_socket(buf_t *buf, tor_socket_t s, size_t at_most, readlen = cap; } - r = read_to_chunk(buf, chunk, s, readlen, reached_eof, socket_error); + r = read_to_chunk(buf, chunk, s, readlen, reached_eof, socket_error, true); check(); if (r < 0) return r; /* Error */

1 0

[tor/master] Refactor buf_read_from_socket() into buf_read_from_fd().
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit 771930b84cb4aeeb75e175368108697bafd94a2b Author: Alexander Færøy <ahf(a)torproject.org> Date: Mon Sep 10 14:35:38 2018 +0200 Refactor buf_read_from_socket() into buf_read_from_fd(). This patch refactors buf_read_from_socket() into buf_read_from_fd(), and creates a specialized function for buf_read_from_socket(), which uses buf_read_from_fd(). See: https://bugs.torproject.org/28179 --- src/lib/net/buffers_net.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/lib/net/buffers_net.c b/src/lib/net/buffers_net.c index 2ffca8b7d..7c81096e3 100644 --- a/src/lib/net/buffers_net.c +++ b/src/lib/net/buffers_net.c @@ -75,16 +75,17 @@ read_to_chunk(buf_t *buf, chunk_t *chunk, tor_socket_t fd, size_t at_most, } } -/** Read from socket s, writing onto end of buf. Read at most - * at_most bytes, growing the buffer as necessary. If recv() returns 0 - * (because of EOF), set *reached_eof to 1 and return 0. Return -1 on - * error; else return the number of bytes read. +/** Read from file descriptor fd, writing onto end of buf. Read + * at most at_most bytes, growing the buffer as necessary. If recv() + * returns 0 (because of EOF), set *reached_eof to 1 and return 0. + * Return -1 on error; else return the number of bytes read. */ /* XXXX indicate "read blocked" somehow? */ -int -buf_read_from_socket(buf_t *buf, tor_socket_t s, size_t at_most, - int *reached_eof, - int *socket_error) +static int +buf_read_from_fd(buf_t *buf, int fd, size_t at_most, + int *reached_eof, + int *socket_error, + bool is_socket) { /* XXXX It's stupid to overload the return values for these functions: * "error status" and "number of bytes read" are not mutually exclusive. @@ -94,7 +95,7 @@ buf_read_from_socket(buf_t *buf, tor_socket_t s, size_t at_most, check(); tor_assert(reached_eof); - tor_assert(SOCKET_OK(s)); + tor_assert(SOCKET_OK(fd)); if (BUG(buf->datalen >= INT_MAX)) return -1; @@ -115,7 +116,8 @@ buf_read_from_socket(buf_t *buf, tor_socket_t s, size_t at_most, readlen = cap; } - r = read_to_chunk(buf, chunk, s, readlen, reached_eof, socket_error, true); + r = read_to_chunk(buf, chunk, fd, readlen, + reached_eof, socket_error, is_socket); check(); if (r < 0) return r; /* Error */ @@ -224,3 +226,16 @@ buf_flush_to_socket(buf_t *buf, tor_socket_t s, size_t sz, { return buf_flush_to_fd(buf, s, sz, buf_flushlen, true); } + +/** Read from socket s, writing onto end of buf. Read at most + * at_most bytes, growing the buffer as necessary. If recv() returns 0 + * (because of EOF), set *reached_eof to 1 and return 0. Return -1 on + * error; else return the number of bytes read. + */ +int +buf_read_from_socket(buf_t *buf, tor_socket_t s, size_t at_most, + int *reached_eof, + int *socket_error) +{ + return buf_read_from_fd(buf, s, at_most, reached_eof, socket_error, true); +}

1 0

[tor/master] Refactor buf_flush_to_socket() into buf_flush_to_fd().
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit c71f9df07bb5f9f01a47a66f245c7794cb2bd839 Author: Alexander Færøy <ahf(a)torproject.org> Date: Mon Sep 10 14:29:27 2018 +0200 Refactor buf_flush_to_socket() into buf_flush_to_fd(). This patch refactors buf_flush_to_socket() into buf_flush_to_fd() and creates a specialization function for buf_flush_to_socket() that makes use of buf_flush_to_fd(). See: https://bugs.torproject.org/28179 --- src/lib/net/buffers_net.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/lib/net/buffers_net.c b/src/lib/net/buffers_net.c index fc133a01f..2ffca8b7d 100644 --- a/src/lib/net/buffers_net.c +++ b/src/lib/net/buffers_net.c @@ -166,15 +166,15 @@ flush_chunk(tor_socket_t fd, buf_t *buf, chunk_t *chunk, size_t sz, } } -/** Write data from buf to the socket s. Write at most +/** Write data from buf to the file descriptor fd. Write at most * sz bytes, decrement *buf_flushlen by * the number of bytes actually written, and remove the written bytes * from the buffer. Return the number of bytes written on success, * -1 on failure. Return 0 if write() would block. */ -int -buf_flush_to_socket(buf_t *buf, tor_socket_t s, size_t sz, - size_t *buf_flushlen) +static int +buf_flush_to_fd(buf_t *buf, int fd, size_t sz, + size_t *buf_flushlen, bool is_socket) { /* XXXX It's stupid to overload the return values for these functions: * "error status" and "number of bytes flushed" are not mutually exclusive. @@ -182,7 +182,7 @@ buf_flush_to_socket(buf_t *buf, tor_socket_t s, size_t sz, int r; size_t flushed = 0; tor_assert(buf_flushlen); - tor_assert(SOCKET_OK(s)); + tor_assert(SOCKET_OK(fd)); if (BUG(*buf_flushlen > buf->datalen)) { *buf_flushlen = buf->datalen; } @@ -199,7 +199,7 @@ buf_flush_to_socket(buf_t *buf, tor_socket_t s, size_t sz, else flushlen0 = buf->head->datalen; - r = flush_chunk(s, buf, buf->head, flushlen0, buf_flushlen, true); + r = flush_chunk(fd, buf, buf->head, flushlen0, buf_flushlen, is_socket); check(); if (r < 0) return r; @@ -211,3 +211,16 @@ buf_flush_to_socket(buf_t *buf, tor_socket_t s, size_t sz, tor_assert(flushed < INT_MAX); return (int)flushed; } + +/** Write data from buf to the socket s. Write at most + * sz bytes, decrement *buf_flushlen by + * the number of bytes actually written, and remove the written bytes + * from the buffer. Return the number of bytes written on success, + * -1 on failure. Return 0 if write() would block. + */ +int +buf_flush_to_socket(buf_t *buf, tor_socket_t s, size_t sz, + size_t *buf_flushlen) +{ + return buf_flush_to_fd(buf, s, sz, buf_flushlen, true); +}

1 0

[tor/master] Remove unused `int pid` member of `managed_proxy_t`.
by nickm＠torproject.org 18 Dec '18

18 Dec '18

commit 2a3eef4404415f1bb90a86c9cc396b6dee89039c Author: Alexander Færøy <ahf(a)torproject.org> Date: Mon Sep 10 13:05:01 2018 +0200 Remove unused `int pid` member of `managed_proxy_t`. See: https://bugs.torproject.org/28179 --- src/feature/client/transports.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index d304dcd48..b80875c95 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -97,8 +97,6 @@ typedef struct { /* A pointer to the process handle of this managed proxy. */ struct process_handle_t *process_handle; - int pid; /* The Process ID this managed proxy is using. */ - /** Boolean: We are re-parsing our config, and we are going to * remove this managed proxy if we don't find it any transport * plugins that use it. */

1 0