- tor-commits - lists.torproject.org

[tor/master] Prevent an (impossible) null-pointer dereference in connection_edge_process_relay_cell
by nickm＠torproject.org 31 May '12

31 May '12

commit edf0d5b12c5d51e9f82e9c215d3b0386cf4688db Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Mar 31 14:17:41 2012 -0400 Prevent an (impossible) null-pointer dereference in connection_edge_process_relay_cell This would happen if the deliver window could become negative because of an nonexistent connection. (Fortunately, _that_ can't occur, thanks to circuit_consider_sending_sendme. Still, if we change our windowing logic at all, we won't want this to become triggerable.) Fix for bug 5541. Bugfix on 4a66865d, back from 0.0.2pre14. asn found this. Nice catch, asn! --- changes/bug5541 | 8 ++++++++ src/or/relay.c | 8 ++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/changes/bug5541 b/changes/bug5541 new file mode 100644 index 0000000..3cf0692 --- /dev/null +++ b/changes/bug5541 @@ -0,0 +1,8 @@ + o Minor bugfixes: + - Prevent a null-pointer dereference when receiving a data cell + for a nonexistent stream when the circuit in question has an + empty deliver window. We don't believe this is triggerable, + since we don't currently allow deliver windows to become empty, + but the logic is tricky enough that it's better to make the code + robust. Fixes bug 5541; bugfix on 0.0.2pre14. + diff --git a/src/or/relay.c b/src/or/relay.c index b637fad..3c2c81b 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -1103,8 +1103,12 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, (!layer_hint && --circ->deliver_window < 0)) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "(relay data) circ deliver_window below 0. Killing."); - connection_edge_end(conn, END_STREAM_REASON_TORPROTOCOL); - connection_mark_for_close(TO_CONN(conn)); + if (conn) { + /* XXXX Do we actually need to do this? Will killing the circuit + * not send an END and mark the stream for close as appropriate? */ + connection_edge_end(conn, END_STREAM_REASON_TORPROTOCOL); + connection_mark_for_close(TO_CONN(conn)); + } return -END_CIRC_REASON_TORPROTOCOL; } log_debug(domain,"circ deliver_window now %d.", layer_hint ?

1 0

[tor/master] Close the windows file handle after CreateFileMapping; it isn't needed
by nickm＠torproject.org 31 May '12

31 May '12

commit ab1b81e838150ae070b1c6818b9c95faeea56c06 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 23 12:39:05 2012 -0400 Close the windows file handle after CreateFileMapping; it isn't needed I did the changes file; the rest came pseudonymously --- changes/close_file_handle | 4 ++++ src/common/compat.c | 15 ++++++++------- src/common/compat.h | 1 - 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/changes/close_file_handle b/changes/close_file_handle new file mode 100644 index 0000000..128ef81 --- /dev/null +++ b/changes/close_file_handle @@ -0,0 +1,4 @@ + o Minor bugfixes: + - Don't hold a windows file handle open for every file mapping; + the file mapping handle is sufficient. Fix for bug 5951; bugfix on + 0.1.2.1-alpha. diff --git a/src/common/compat.c b/src/common/compat.c index a4e5074..3174da6 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -175,24 +175,24 @@ tor_mmap_file(const char *filename) TCHAR tfilename[MAX_PATH]= {0}; tor_mmap_t *res = tor_malloc_zero(sizeof(tor_mmap_t)); int empty = 0; - res->file_handle = INVALID_HANDLE_VALUE; + HANDLE file_handle = INVALID_HANDLE_VALUE; res->mmap_handle = NULL; #ifdef UNICODE mbstowcs(tfilename,filename,MAX_PATH); #else strlcpy(tfilename,filename,MAX_PATH); #endif - res->file_handle = CreateFile(tfilename, + file_handle = CreateFile(tfilename, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0); - if (res->file_handle == INVALID_HANDLE_VALUE) + if (file_handle == INVALID_HANDLE_VALUE) goto win_err; - res->size = GetFileSize(res->file_handle, NULL); + res->size = GetFileSize(file_handle, NULL); if (res->size == 0) { log_info(LD_FS,"File \"%s\" is empty. Ignoring.",filename); @@ -200,7 +200,7 @@ tor_mmap_file(const char *filename) goto err; } - res->mmap_handle = CreateFileMapping(res->file_handle, + res->mmap_handle = CreateFileMapping(file_handle, NULL, PAGE_READONLY, #if SIZEOF_SIZE_T > 4 @@ -218,6 +218,7 @@ tor_mmap_file(const char *filename) if (!res->data) goto win_err; + CloseHandle(file_handle); return res; win_err: { DWORD e = GetLastError(); @@ -234,6 +235,8 @@ tor_mmap_file(const char *filename) err: if (empty) errno = ERANGE; + if (file_handle != INVALID_HANDLE_VALUE) + CloseHandle(file_handle); tor_munmap_file(res); return NULL; } @@ -247,8 +250,6 @@ tor_munmap_file(tor_mmap_t *handle) if (handle->mmap_handle != NULL) CloseHandle(handle->mmap_handle); - if (handle->file_handle != INVALID_HANDLE_VALUE) - CloseHandle(handle->file_handle); tor_free(handle); } #else diff --git a/src/common/compat.h b/src/common/compat.h index d2f1fd1..06aeeb1 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -249,7 +249,6 @@ typedef struct tor_mmap_t { size_t mapping_size; /**< Size of the actual mapping. (This is this file * size, rounded up to the nearest page.) */ #elif defined MS_WINDOWS - HANDLE file_handle; HANDLE mmap_handle; #endif

1 0

[tor/master] reindent CreateFile arguments.
by nickm＠torproject.org 31 May '12

31 May '12

commit 2e58882b90a5015554b59862869b20dba39a5d58 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 31 12:36:54 2012 -0400 reindent CreateFile arguments. --- src/common/compat.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index 3174da6..82332f1 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -183,11 +183,11 @@ tor_mmap_file(const char *filename) strlcpy(tfilename,filename,MAX_PATH); #endif file_handle = CreateFile(tfilename, - GENERIC_READ, FILE_SHARE_READ, - NULL, - OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, - 0); + GENERIC_READ, FILE_SHARE_READ, + NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + 0); if (file_handle == INVALID_HANDLE_VALUE) goto win_err;

1 0

[tor/master] Merge remote-tracking branch 'public/close_file_mapping'
by nickm＠torproject.org 31 May '12

31 May '12

commit 0e207f9acb655d037c8aef86ea2781d8e1b4b570 Merge: f1aae12 2e58882 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 31 12:38:11 2012 -0400 Merge remote-tracking branch 'public/close_file_mapping' Conflicts: src/common/compat.h Conflict was between replacement of MS_WINDOWS with _WIN32 in master, and with removal of file_handle from tor_mmap_t struct in close_file_mapping branch (for bug 5951 fix). changes/close_file_handle | 4 ++++ src/common/compat.c | 25 +++++++++++++------------ src/common/compat.h | 1 - 3 files changed, 17 insertions(+), 13 deletions(-) diff --cc src/common/compat.h index 7edd889,06aeeb1..7ba3509 --- a/src/common/compat.h +++ b/src/common/compat.h @@@ -246,8 -248,7 +246,7 @@@ typedef struct tor_mmap_t #ifdef HAVE_SYS_MMAN_H size_t mapping_size; /**< Size of the actual mapping. (This is this file * size, rounded up to the nearest page.) */ -#elif defined MS_WINDOWS +#elif defined _WIN32 - HANDLE file_handle; HANDLE mmap_handle; #endif

1 0

[tor/master] Refactor new getcwd code
by nickm＠torproject.org 31 May '12

31 May '12

commit 57ed459b0d675d255ae452749bee9efa3c498a5b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 10 14:20:15 2012 -0400 Refactor new getcwd code Make sure that the "path_length *= 2" statement can't overflow. Move the "malloc and getcwd" loop into its own function. --- src/common/compat.c | 65 +++++++++++++++++++++++++++++++++----------------- 1 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index ec365c3..6c833f1 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1630,6 +1630,38 @@ get_parent_directory(char *fname) return -1; } +#ifndef _WIN32 +/** Return a newly allocated string containing the output of getcwd(). Return + * NULL on failure. (We can't just use getcwd() into a PATH_MAX buffer, since + * Hurd hasn't got a PATH_MAX.) + */ +static char * +alloc_getcwd(void) +{ + int saved_errno = errno; +/* We use this as a starting path length. Not too large seems sane. */ +#define START_PATH_LENGTH 128 +/* Nobody has a maxpath longer than this, as far as I know. And if they + * do, they shouldn't. */ +#define MAX_SANE_PATH_LENGTH 4096 + size_t path_length = START_PATH_LENGTH; + char *path = tor_malloc(path_length); + + errno = 0; + while (getcwd(path, path_length) == NULL) { + if (errno == ERANGE && path_length < MAX_SANE_PATH_LENGTH) { + path_length*=2; + path = tor_realloc(path, path_length); + } else { + tor_free(path); + return NULL; + } + } + errno = saved_errno; + return path; +} +#endif + /** Expand possibly relative path <b>fname</b> to an absolute path. * Return a newly allocated string, possibly equal to <b>fname</b>. */ char * @@ -1645,36 +1677,25 @@ make_path_absolute(char *fname) return absfname; #else -/* We use this as a starting path length. Not too large seems sane. */ -#define START_PATH_LENGTH 100 - size_t path_length = START_PATH_LENGTH; - char *path = tor_malloc(path_length); - char *absfname = NULL; + char *absfname = NULL, *path = NULL; tor_assert(fname); if (fname[0] == '/') { absfname = tor_strdup(fname); } else { - int save_errno = errno; - errno = 0; - while (getcwd(path, path_length) == NULL) { - if (errno == ERANGE) { - path_length*=2; - path = tor_realloc(path, path_length); - } else { - /* If getcwd failed with an error other than ERANGE, the best we can - * do here is keep using the relative path. (Perhaps / isn't readable - * by this UID/GID.) */ - absfname = tor_strdup(fname); - break; - } + path = alloc_getcwd(); + if (path) { + tor_asprintf(&absfname, "%s/%s", path, fname); + tor_free(path); + } else { + /* If getcwd failed, the best we can do here is keep using the + * relative path. (Perhaps / isn't readable by this UID/GID.) */ + log_warn(LD_GENERAL, "Unable to find current working directory: %s", + strerror(errno)); + absfname = tor_strdup(fname); } - errno = save_errno; - tor_asprintf(&absfname, "%s/%s", path, fname); - tor_free(path); } - return absfname; #endif }

1 0

[tor/master] Stop using MAX_PATH, it might not be defined
by nickm＠torproject.org 31 May '12

31 May '12

commit d916fc38b6386c59f3e405e2f3b35e1d505fd806 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Sat Mar 10 16:53:01 2012 +0100 Stop using MAX_PATH, it might not be defined This broke compilation on Hurd --- changes/bug5355 | 4 ++++ src/common/compat.c | 27 ++++++++++++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/changes/bug5355 b/changes/bug5355 new file mode 100644 index 0000000..f850fe1 --- /dev/null +++ b/changes/bug5355 @@ -0,0 +1,4 @@ + o Major bugfixes: + - Fix a compilation issue on GNU Hurd, which doesn't have PATH_MAX. Fixes + bug 5355; bugfix on 0.2.3.11-alpha. + diff --git a/src/common/compat.c b/src/common/compat.c index 30bde3d..ec365c3 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1645,7 +1645,10 @@ make_path_absolute(char *fname) return absfname; #else - char path[PATH_MAX+1]; +/* We use this as a starting path length. Not too large seems sane. */ +#define START_PATH_LENGTH 100 + size_t path_length = START_PATH_LENGTH; + char *path = tor_malloc(path_length); char *absfname = NULL; tor_assert(fname); @@ -1653,13 +1656,23 @@ make_path_absolute(char *fname) if (fname[0] == '/') { absfname = tor_strdup(fname); } else { - if (getcwd(path, PATH_MAX) != NULL) { - tor_asprintf(&absfname, "%s/%s", path, fname); - } else { - /* If getcwd failed, the best we can do here is keep using the - * relative path. (Perhaps / isn't readable by this UID/GID.) */ - absfname = tor_strdup(fname); + int save_errno = errno; + errno = 0; + while (getcwd(path, path_length) == NULL) { + if (errno == ERANGE) { + path_length*=2; + path = tor_realloc(path, path_length); + } else { + /* If getcwd failed with an error other than ERANGE, the best we can + * do here is keep using the relative path. (Perhaps / isn't readable + * by this UID/GID.) */ + absfname = tor_strdup(fname); + break; + } } + errno = save_errno; + tor_asprintf(&absfname, "%s/%s", path, fname); + tor_free(path); } return absfname;

1 0

[tor/master] Merge remote-tracking branch 'linus/bug5355_ln'
by nickm＠torproject.org 31 May '12

31 May '12

commit f1aae1236fc68a0047ac9370bdcf828a42efdab3 Merge: 155543d f998590 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 31 12:33:16 2012 -0400 Merge remote-tracking branch 'linus/bug5355_ln' changes/bug5355 | 4 ++++ src/common/compat.c | 43 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 4 deletions(-)

1 0

[tor/master] Don't stomp on errno.
by nickm＠torproject.org 31 May '12

31 May '12

commit f998590e5b02ca7e0d2224c600a877c93a3ab2e9 Author: Linus Nordberg <linus(a)nordberg.se> Date: Tue May 29 15:38:03 2012 +0200 Don't stomp on errno. --- src/common/compat.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index 6c833f1..c18b417 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1654,7 +1654,8 @@ alloc_getcwd(void) path = tor_realloc(path, path_length); } else { tor_free(path); - return NULL; + path = NULL; + break; } } errno = saved_errno;

1 0

[tor/master] When downloading bridge descs from a bridge authority, always be anonymous
by nickm＠torproject.org 31 May '12

31 May '12

commit 8f070ecbc000c3ca32ff1467716f777a44d0eda4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 27 11:27:32 2012 -0400 When downloading bridge descs from a bridge authority, always be anonymous --- changes/bug1938 | 4 ++++ src/or/directory.c | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/changes/bug1938 b/changes/bug1938 new file mode 100644 index 0000000..13d2a37 --- /dev/null +++ b/changes/bug1938 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - When fetching a bridge descriptor from a bridge authority, + always do so anonymously, whether we have been able to open + circuits or not. Partial fix for bug 1938; bugfix on 2.0.7-alpha. diff --git a/src/or/directory.c b/src/or/directory.c index 6394aec..3d2b955 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -130,8 +130,9 @@ purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose) { if (get_options()->AllDirActionsPrivate) return 1; - if (router_purpose == ROUTER_PURPOSE_BRIDGE && can_complete_circuit) - return 1; /* if no circuits yet, we may need this info to bootstrap. */ + if (router_purpose == ROUTER_PURPOSE_BRIDGE) + return 1; /* if no circuits yet, this might break bootstrapping, but it's + * needed to be safe. */ if (dir_purpose == DIR_PURPOSE_UPLOAD_DIR || dir_purpose == DIR_PURPOSE_UPLOAD_VOTE || dir_purpose == DIR_PURPOSE_UPLOAD_SIGNATURES ||

1 0

[tor/master] Merge remote-tracking branch 'public/bug1938'
by nickm＠torproject.org 31 May '12

31 May '12

commit 155543d26e21f992237c2fff159869cc663ae049 Merge: 11bf558 8f070ec Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 31 12:24:02 2012 -0400 Merge remote-tracking branch 'public/bug1938' changes/bug1938 | 4 ++++ src/or/directory.c | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-)

1 0