August 2012 - tor-commits - lists.torproject.org

[tor/master] Remove needless flush-on-write code.
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit d9746bd468f551d1ada57d962b20eddd15256ce9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 25 18:34:14 2012 -0400 Remove needless flush-on-write code. Long ago, before we had cell queues, it was necessary to maybe call connection_handle_write() from connectino_write_to_buf_impl() on OR connections, so that we wouldn't get into a loop of reading infinite amounts of data and queueing it all on an outbuf before bothering to write any data. If that doesn't sounds like what our code does now, you're right: right now, we won't stick more than OR_CONN_HIGHWATER bytes of cells on an outbuf, and we won't suck more than CELL_QUEUE_HIGHWATER_SIZE cells off any edge connection. So, there's no more call for that code. Removing this code will simplify our data flow, and that should be something we can all get behind. --- changes/bug6468 | 8 ++++++++ src/or/connection.c | 28 ++++------------------------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/changes/bug6468 b/changes/bug6468 new file mode 100644 index 0000000..fb624da --- /dev/null +++ b/changes/bug6468 @@ -0,0 +1,8 @@ + o Minor bugfixes: + - Remove some now-needless code that tried to aggressively flush + OR connections as data was added to them. Since 0.2.0.1-alpha, + our cell queue logic has saved us from the failure mode that + this code was supposed to prevent. Removing this code will limit + the number of baroque control flow paths through Tor's network + logic. Reported pseudonymously on IRC. Fix for bug 6468; + bigfix on 0.2.0.1-alpha. \ No newline at end of file diff --git a/src/or/connection.c b/src/or/connection.c index a1d5354..c224496 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -3365,13 +3365,6 @@ connection_flush(connection_t *conn) return connection_handle_write(conn, 1); } -/** OpenSSL TLS record size is 16383; this is close. The goal here is to - * push data out as soon as we know there's enough for a TLS record, so - * during periods of high load we won't read entire megabytes from - * input before pushing any data out. It also has the feature of not - * growing huge outbufs unless something is slow. */ -#define MIN_TLS_FLUSHLEN 15872 - /** Append len bytes of string onto conn's * outbuf, and ask it to start writing. * @@ -3380,10 +3373,9 @@ connection_flush(connection_t *conn) * negative, this is the last data to be compressed, and the connection's zlib * state should be flushed. * - * If it's an OR conn and an entire TLS record is ready, then try to - * flush the record now. Similarly, if it's a local control connection - * and a 64k chunk is ready, try to flush it all, so we don't end up with - * many megabytes of controller info queued at once. + * If it's a local control connection and a 64k chunk is ready, try to flush + * it all, so we don't end up with many megabytes of controller info queued at + * once. */ void _connection_write_to_buf_impl(const char *string, size_t len, @@ -3451,7 +3443,6 @@ _connection_write_to_buf_impl(const char *string, size_t len, if (zlib) { conn->outbuf_flushlen += buf_datalen(conn->outbuf) - old_datalen; } else { - ssize_t extra = 0; conn->outbuf_flushlen += len; /* Should we try flushing the outbuf now? */ @@ -3461,14 +3452,7 @@ _connection_write_to_buf_impl(const char *string, size_t len, return; } - if (conn->type == CONN_TYPE_OR && - conn->outbuf_flushlen-len < MIN_TLS_FLUSHLEN && - conn->outbuf_flushlen >= MIN_TLS_FLUSHLEN) { - /* We just pushed outbuf_flushlen to MIN_TLS_FLUSHLEN or above; - * we can send out a full TLS frame now if we like. */ - extra = conn->outbuf_flushlen - MIN_TLS_FLUSHLEN; - conn->outbuf_flushlen = MIN_TLS_FLUSHLEN; - } else if (conn->type == CONN_TYPE_CONTROL && + if (conn->type == CONN_TYPE_CONTROL && !connection_is_rate_limited(conn) && conn->outbuf_flushlen-len < 1<<16 && conn->outbuf_flushlen >= 1<<16) { @@ -3488,10 +3472,6 @@ _connection_write_to_buf_impl(const char *string, size_t len, } return; } - if (extra) { - conn->outbuf_flushlen += extra; - connection_start_writing(conn); - } } }

1 0

[tor/master] Consider IPv6 OR ports when deciding whether a routerinfo change is cosmetic.
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 9ed87b37d04705983b11cd995140e8864454bf5e Author: Linus Nordberg <linus(a)torproject.org> Date: Thu Jul 19 20:41:23 2012 +0200 Consider IPv6 OR ports when deciding whether a routerinfo change is cosmetic. Closes #6423. --- changes/bug6423 | 3 +++ src/or/routerlist.c | 2 ++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/changes/bug6423 b/changes/bug6423 new file mode 100644 index 0000000..2ea4f14 --- /dev/null +++ b/changes/bug6423 @@ -0,0 +1,3 @@ + o Minor features: + - Consider new, removed or changed IPv6 OR ports a non cosmetic + change. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 4979b93..3c39e36 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5160,6 +5160,8 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2) if (strcasecmp(r1->address, r2->address) || strcasecmp(r1->nickname, r2->nickname) || r1->or_port != r2->or_port || + !tor_addr_eq(&r1->ipv6_addr, &r2->ipv6_addr) || + r1->ipv6_orport != r2->ipv6_orport || r1->dir_port != r2->dir_port || r1->purpose != r2->purpose || crypto_pk_cmp_keys(r1->onion_pkey, r2->onion_pkey) ||

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.3'
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit a9d56289ee53da6aa98a0c8b89796b7872ff6e1a Merge: 851197f 9ed87b3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 15:59:40 2012 -0400 Merge remote-tracking branch 'origin/maint-0.2.3' changes/bug6423 | 3 +++ src/or/routerlist.c | 2 ++ 2 files changed, 5 insertions(+), 0 deletions(-)

1 0

[tor/maint-0.2.3] Consider IPv6 OR ports when deciding whether a routerinfo change is cosmetic.
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 9ed87b37d04705983b11cd995140e8864454bf5e Author: Linus Nordberg <linus(a)torproject.org> Date: Thu Jul 19 20:41:23 2012 +0200 Consider IPv6 OR ports when deciding whether a routerinfo change is cosmetic. Closes #6423. --- changes/bug6423 | 3 +++ src/or/routerlist.c | 2 ++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/changes/bug6423 b/changes/bug6423 new file mode 100644 index 0000000..2ea4f14 --- /dev/null +++ b/changes/bug6423 @@ -0,0 +1,3 @@ + o Minor features: + - Consider new, removed or changed IPv6 OR ports a non cosmetic + change. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 4979b93..3c39e36 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5160,6 +5160,8 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2) if (strcasecmp(r1->address, r2->address) || strcasecmp(r1->nickname, r2->nickname) || r1->or_port != r2->or_port || + !tor_addr_eq(&r1->ipv6_addr, &r2->ipv6_addr) || + r1->ipv6_orport != r2->ipv6_orport || r1->dir_port != r2->dir_port || r1->purpose != r2->purpose || crypto_pk_cmp_keys(r1->onion_pkey, r2->onion_pkey) ||

1 0

[tor/master] Fix more warnings from openbsd_malloc
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 4c8fcba86c35bd8e2cfde4c65f394c7837638ff0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 13:49:52 2012 -0400 Fix more warnings from openbsd_malloc Apparently, (void)writev is not enough to suppress the "you are ignoring the return value!" warnings on Linux. Instead, remove the whole warning/error logic when compiling openbsd_malloc for Tor: we can't use it. --- src/common/OpenBSD_malloc_Linux.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/common/OpenBSD_malloc_Linux.c b/src/common/OpenBSD_malloc_Linux.c index 39c8f8b..da82729 100644 --- a/src/common/OpenBSD_malloc_Linux.c +++ b/src/common/OpenBSD_malloc_Linux.c @@ -450,6 +450,7 @@ extern char *__progname; static void wrterror(const char *p) { +#ifndef BUILDING_FOR_TOR const char *q = " error: "; struct iovec iov[5]; @@ -463,8 +464,10 @@ wrterror(const char *p) iov[3].iov_len = strlen(p); iov[4].iov_base = (char*)"\n"; iov[4].iov_len = 1; - (void) writev(STDERR_FILENO, iov, 5); - + writev(STDERR_FILENO, iov, 5); +#else + (void)p; +#endif suicide = 1; #ifdef MALLOC_STATS if (malloc_stats) @@ -478,14 +481,17 @@ wrterror(const char *p) static void wrtwarning(const char *p) { +#ifndef BUILDING_FOR_TOR const char *q = " warning: "; struct iovec iov[5]; +#endif if (malloc_abort) wrterror(p); else if (malloc_silent) return; +#ifndef BUILDING_FOR_TOR iov[0].iov_base = __progname; iov[0].iov_len = strlen(__progname); iov[1].iov_base = (char*)malloc_func; @@ -498,6 +504,9 @@ wrtwarning(const char *p) iov[4].iov_len = 1; (void) writev(STDERR_FILENO, iov, 5); +#else + (void)p; +#endif } #ifdef MALLOC_STATS

1 0

[tor/master] Fix warnings and 64-bit problems in openbsd-malloc code
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 6a33c33a12f3af414e97b91d5718ea16f2d26673 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 15 19:26:53 2012 -0400 Fix warnings and 64-bit problems in openbsd-malloc code The warning fixes are: - Only define issetugid if it's missing. - Explicitly ignore the return value of writev. - Explicitly cast the retval of readlink() to int. The 64-bit problems are related to just storing a size_t in an int. Not cool! Use a size_t instead. Fix for bug 6379. Bugfix on 0.2.0.20-rc, which introduced openbsd-malloc. --- changes/bug6379 | 6 ++++++ configure.in | 1 + src/common/OpenBSD_malloc_Linux.c | 31 ++++++++++++++++++++++--------- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/changes/bug6379 b/changes/bug6379 new file mode 100644 index 0000000..1f2b694 --- /dev/null +++ b/changes/bug6379 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Fix build warnings from --enable-openbsd-malloc with gcc warnings + enabled. Fixes bug 6379. + - Fix 64-bit warnings from --enable-openbsd-malloc. Fixes bug 6379. + Bugfix on 0.2.0.20-rc. + diff --git a/configure.in b/configure.in index 1db7d08..8aa90f6 100644 --- a/configure.in +++ b/configure.in @@ -300,6 +300,7 @@ AC_CHECK_FUNCS( gmtime_r \ inet_aton \ ioctl \ + issetugid \ localtime_r \ lround \ memmem \ diff --git a/src/common/OpenBSD_malloc_Linux.c b/src/common/OpenBSD_malloc_Linux.c index 92ca9c0..39c8f8b 100644 --- a/src/common/OpenBSD_malloc_Linux.c +++ b/src/common/OpenBSD_malloc_Linux.c @@ -14,6 +14,10 @@ * ---------------------------------------------------------------------------- */ +/* We use this macro to remove some code that we don't actually want, + * rather than to fix its warnings. */ +#define BUILDING_FOR_TOR + /* * Defining MALLOC_EXTRA_SANITY will enable extra checks which are * related to internal conditions and consistency in malloc.c. This has @@ -79,6 +83,7 @@ static size_t g_alignment = 0; extern int __libc_enable_secure; +#ifndef HAVE_ISSETUGID static int issetugid(void) { if (__libc_enable_secure) return 1; @@ -86,8 +91,10 @@ static int issetugid(void) if (getgid() != getegid()) return 1; return 0; } +#endif #define PGSHIFT 12 +#undef MADV_FREE #define MADV_FREE MADV_DONTNEED #include <pthread.h> static pthread_mutex_t gen_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -456,7 +463,7 @@ wrterror(const char *p) iov[3].iov_len = strlen(p); iov[4].iov_base = (char*)"\n"; iov[4].iov_len = 1; - writev(STDERR_FILENO, iov, 5); + (void) writev(STDERR_FILENO, iov, 5); suicide = 1; #ifdef MALLOC_STATS @@ -489,8 +496,8 @@ wrtwarning(const char *p) iov[3].iov_len = strlen(p); iov[4].iov_base = (char*)"\n"; iov[4].iov_len = 1; - - writev(STDERR_FILENO, iov, 5); + + (void) writev(STDERR_FILENO, iov, 5); } #ifdef MALLOC_STATS @@ -665,7 +672,7 @@ malloc_init(void) for (i = 0; i < 3; i++) { switch (i) { case 0: - j = readlink("/etc/malloc.conf", b, sizeof b - 1); + j = (int) readlink("/etc/malloc.conf", b, sizeof b - 1); if (j <= 0) continue; b[j] = '\0'; @@ -1145,9 +1152,10 @@ malloc_bytes(size_t size) if (size == 0) j = 0; else { + size_t ii; j = 1; - i = size - 1; - while (i >>= 1) + ii = size - 1; + while (ii >>= 1) j++; } @@ -1971,6 +1979,7 @@ calloc(size_t num, size_t size) return(p); } +#ifndef BUILDING_FOR_TOR static int ispowerof2 (size_t a) { size_t b; for (b = 1ULL << (sizeof(size_t)*NBBY - 1); b > 1; b >>= 1) @@ -1978,7 +1987,9 @@ static int ispowerof2 (size_t a) { return 1; return 0; } +#endif +#ifndef BUILDING_FOR_TOR int posix_memalign(void **memptr, size_t alignment, size_t size) { void *r; @@ -2015,18 +2026,20 @@ void *valloc(size_t size) posix_memalign(&r, malloc_pagesize, size); return r; } +#endif size_t malloc_good_size(size_t size) { if (size == 0) { return 1; } else if (size <= malloc_maxsize) { - int i, j; + int j; + size_t ii; /* round up to the nearest power of 2, with same approach * as malloc_bytes() uses. */ j = 1; - i = size - 1; - while (i >>= 1) + ii = size - 1; + while (ii >>= 1) j++; return ((size_t)1) << j; } else {

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.3'
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 851197f11d8d602de5c6487d79a21339a587ebb6 Merge: eec8693 97602c9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 14:04:28 2012 -0400 Merge remote-tracking branch 'origin/maint-0.2.3' changes/bug6379 | 6 +++++ configure.in | 1 + src/common/OpenBSD_malloc_Linux.c | 40 ++++++++++++++++++++++++++++-------- 3 files changed, 38 insertions(+), 9 deletions(-)

1 0

[tor/master] Merge branch 'bug6379' into maint-0.2.3
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 97602c9de432cc1d49d4441de858ab73b8bf41c0 Merge: e9172e5 4c8fcba Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 13:57:07 2012 -0400 Merge branch 'bug6379' into maint-0.2.3 changes/bug6379 | 6 +++++ configure.in | 1 + src/common/OpenBSD_malloc_Linux.c | 40 ++++++++++++++++++++++++++++-------- 3 files changed, 38 insertions(+), 9 deletions(-)

1 0

[tor/maint-0.2.3] Fix warnings and 64-bit problems in openbsd-malloc code
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 6a33c33a12f3af414e97b91d5718ea16f2d26673 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 15 19:26:53 2012 -0400 Fix warnings and 64-bit problems in openbsd-malloc code The warning fixes are: - Only define issetugid if it's missing. - Explicitly ignore the return value of writev. - Explicitly cast the retval of readlink() to int. The 64-bit problems are related to just storing a size_t in an int. Not cool! Use a size_t instead. Fix for bug 6379. Bugfix on 0.2.0.20-rc, which introduced openbsd-malloc. --- changes/bug6379 | 6 ++++++ configure.in | 1 + src/common/OpenBSD_malloc_Linux.c | 31 ++++++++++++++++++++++--------- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/changes/bug6379 b/changes/bug6379 new file mode 100644 index 0000000..1f2b694 --- /dev/null +++ b/changes/bug6379 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Fix build warnings from --enable-openbsd-malloc with gcc warnings + enabled. Fixes bug 6379. + - Fix 64-bit warnings from --enable-openbsd-malloc. Fixes bug 6379. + Bugfix on 0.2.0.20-rc. + diff --git a/configure.in b/configure.in index 1db7d08..8aa90f6 100644 --- a/configure.in +++ b/configure.in @@ -300,6 +300,7 @@ AC_CHECK_FUNCS( gmtime_r \ inet_aton \ ioctl \ + issetugid \ localtime_r \ lround \ memmem \ diff --git a/src/common/OpenBSD_malloc_Linux.c b/src/common/OpenBSD_malloc_Linux.c index 92ca9c0..39c8f8b 100644 --- a/src/common/OpenBSD_malloc_Linux.c +++ b/src/common/OpenBSD_malloc_Linux.c @@ -14,6 +14,10 @@ * ---------------------------------------------------------------------------- */ +/* We use this macro to remove some code that we don't actually want, + * rather than to fix its warnings. */ +#define BUILDING_FOR_TOR + /* * Defining MALLOC_EXTRA_SANITY will enable extra checks which are * related to internal conditions and consistency in malloc.c. This has @@ -79,6 +83,7 @@ static size_t g_alignment = 0; extern int __libc_enable_secure; +#ifndef HAVE_ISSETUGID static int issetugid(void) { if (__libc_enable_secure) return 1; @@ -86,8 +91,10 @@ static int issetugid(void) if (getgid() != getegid()) return 1; return 0; } +#endif #define PGSHIFT 12 +#undef MADV_FREE #define MADV_FREE MADV_DONTNEED #include <pthread.h> static pthread_mutex_t gen_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -456,7 +463,7 @@ wrterror(const char *p) iov[3].iov_len = strlen(p); iov[4].iov_base = (char*)"\n"; iov[4].iov_len = 1; - writev(STDERR_FILENO, iov, 5); + (void) writev(STDERR_FILENO, iov, 5); suicide = 1; #ifdef MALLOC_STATS @@ -489,8 +496,8 @@ wrtwarning(const char *p) iov[3].iov_len = strlen(p); iov[4].iov_base = (char*)"\n"; iov[4].iov_len = 1; - - writev(STDERR_FILENO, iov, 5); + + (void) writev(STDERR_FILENO, iov, 5); } #ifdef MALLOC_STATS @@ -665,7 +672,7 @@ malloc_init(void) for (i = 0; i < 3; i++) { switch (i) { case 0: - j = readlink("/etc/malloc.conf", b, sizeof b - 1); + j = (int) readlink("/etc/malloc.conf", b, sizeof b - 1); if (j <= 0) continue; b[j] = '\0'; @@ -1145,9 +1152,10 @@ malloc_bytes(size_t size) if (size == 0) j = 0; else { + size_t ii; j = 1; - i = size - 1; - while (i >>= 1) + ii = size - 1; + while (ii >>= 1) j++; } @@ -1971,6 +1979,7 @@ calloc(size_t num, size_t size) return(p); } +#ifndef BUILDING_FOR_TOR static int ispowerof2 (size_t a) { size_t b; for (b = 1ULL << (sizeof(size_t)*NBBY - 1); b > 1; b >>= 1) @@ -1978,7 +1987,9 @@ static int ispowerof2 (size_t a) { return 1; return 0; } +#endif +#ifndef BUILDING_FOR_TOR int posix_memalign(void **memptr, size_t alignment, size_t size) { void *r; @@ -2015,18 +2026,20 @@ void *valloc(size_t size) posix_memalign(&r, malloc_pagesize, size); return r; } +#endif size_t malloc_good_size(size_t size) { if (size == 0) { return 1; } else if (size <= malloc_maxsize) { - int i, j; + int j; + size_t ii; /* round up to the nearest power of 2, with same approach * as malloc_bytes() uses. */ j = 1; - i = size - 1; - while (i >>= 1) + ii = size - 1; + while (ii >>= 1) j++; return ((size_t)1) << j; } else {

1 0

[tor/maint-0.2.3] Merge branch 'bug6379' into maint-0.2.3
by nickm＠torproject.org 17 Aug '12

17 Aug '12

commit 97602c9de432cc1d49d4441de858ab73b8bf41c0 Merge: e9172e5 4c8fcba Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 13:57:07 2012 -0400 Merge branch 'bug6379' into maint-0.2.3 changes/bug6379 | 6 +++++ configure.in | 1 + src/common/OpenBSD_malloc_Linux.c | 40 ++++++++++++++++++++++++++++-------- 3 files changed, 38 insertions(+), 9 deletions(-)

1 0