Fix some 'impossible' overflow bugs in byte counting

The first was genuinely impossible, I think: it could only happen
when the amount we read differed from the amount we wanted to read
by more than INT_MAX.

The second is just very unlikely: it would give incorrect results to
the controller if you somehow wrote or read more than 4GB on one
edge conn in one second.  That one is a bugfix on 0.1.2.8-beta.
This commit is contained in:
Nick Mathewson 2011-03-25 16:45:25 -04:00
parent 6a5b94de6c
commit dddd333a80
2 changed files with 27 additions and 14 deletions

5
changes/count_overflow Normal file
View File

@ -0,0 +1,5 @@
o Minor bugfixes:
- Correctly handle an "impossible" overflow cases in connection
byte counting, where we write or read more than 4GB on an edge
connection in single second. Bugfix on 0.1.2.8-beta.

View File

@ -51,7 +51,7 @@ static int connection_finished_flushing(connection_t *conn);
static int connection_flushed_some(connection_t *conn); static int connection_flushed_some(connection_t *conn);
static int connection_finished_connecting(connection_t *conn); static int connection_finished_connecting(connection_t *conn);
static int connection_reached_eof(connection_t *conn); static int connection_reached_eof(connection_t *conn);
static int connection_read_to_buf(connection_t *conn, int *max_to_read, static int connection_read_to_buf(connection_t *conn, ssize_t *max_to_read,
int *socket_error); int *socket_error);
static int connection_process_inbuf(connection_t *conn, int package_partial); static int connection_process_inbuf(connection_t *conn, int package_partial);
static void client_check_address_changed(int sock); static void client_check_address_changed(int sock);
@ -2338,7 +2338,7 @@ connection_bucket_should_increase(int bucket, or_connection_t *conn)
static int static int
connection_handle_read_impl(connection_t *conn) connection_handle_read_impl(connection_t *conn)
{ {
int max_to_read=-1, try_to_read; ssize_t max_to_read=-1, try_to_read;
size_t before, n_read = 0; size_t before, n_read = 0;
int socket_error = 0; int socket_error = 0;
@ -2456,7 +2456,8 @@ connection_handle_read(connection_t *conn)
* Return -1 if we want to break conn, else return 0. * Return -1 if we want to break conn, else return 0.
*/ */
static int static int
connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error) connection_read_to_buf(connection_t *conn, ssize_t *max_to_read,
int *socket_error)
{ {
int result; int result;
ssize_t at_most = *max_to_read; ssize_t at_most = *max_to_read;
@ -2574,15 +2575,19 @@ connection_read_to_buf(connection_t *conn, int *max_to_read, int *socket_error)
n_read = (size_t) result; n_read = (size_t) result;
} }
if (n_read > 0) { /* change *max_to_read */ if (n_read > 0) {
/*XXXX022 check for overflow*/ /* change *max_to_read */
*max_to_read = (int)(at_most - n_read); *max_to_read = at_most - n_read;
}
if (conn->type == CONN_TYPE_AP) { /* Update edge_conn->n_read */
edge_connection_t *edge_conn = TO_EDGE_CONN(conn); if (conn->type == CONN_TYPE_AP) {
/*XXXX022 check for overflow*/ edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
edge_conn->n_read += (int)n_read; /* Check for overflow: */
if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_read > n_read))
edge_conn->n_read += (int)n_read;
else
edge_conn->n_read = UINT32_MAX;
}
} }
connection_buckets_decrement(conn, approx_time(), n_read, n_written); connection_buckets_decrement(conn, approx_time(), n_read, n_written);
@ -2781,10 +2786,13 @@ connection_handle_write_impl(connection_t *conn, int force)
n_written = (size_t) result; n_written = (size_t) result;
} }
if (conn->type == CONN_TYPE_AP) { if (n_written && conn->type == CONN_TYPE_AP) {
edge_connection_t *edge_conn = TO_EDGE_CONN(conn); edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
/*XXXX022 check for overflow.*/ /* Check for overflow: */
edge_conn->n_written += (int)n_written; if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_written > n_written))
edge_conn->n_written += n_written;
else
edge_conn->n_written = UINT32_MAX;
} }
connection_buckets_decrement(conn, approx_time(), n_read, n_written); connection_buckets_decrement(conn, approx_time(), n_read, n_written);