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.

diff --git a/changes/count_overflow b/changes/count_overflow
new file mode 100644 (file)
index 0000000..f302ff2
--- /dev/null
+++ b/changes/count_overflow
@@ -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.
+

diff --git a/src/or/connection.c b/src/or/connection.c
index 7fa6cd9c175abc63d03638e03e532003ecd966ea..084237dea109c58548badc4f3c1cac0cf0538fc4 100644 (file)
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -51,7 +51,7 @@ static int connection_finished_flushing(connection_t *conn);
 static int connection_flushed_some(connection_t *conn);
 static int connection_finished_connecting(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);
 static int connection_process_inbuf(connection_t *conn, int package_partial);
 static void client_check_address_changed(int sock);
@@ -2338,7 +2338,7 @@ connection_bucket_should_increase(int bucket, or_connection_t *conn)
 static int
 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;
   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.
  */
 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;
   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;
   }
 
-  if (n_read > 0) { /* change *max_to_read */
-    /*XXXX022 check for overflow*/
-    *max_to_read = (int)(at_most - n_read);
-  }
+  if (n_read > 0) {
+    /* change *max_to_read */
+    *max_to_read = at_most - n_read;
 
-  if (conn->type == CONN_TYPE_AP) {
-    edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
-    /*XXXX022 check for overflow*/
-    edge_conn->n_read += (int)n_read;
+    /* Update edge_conn->n_read */
+    if (conn->type == CONN_TYPE_AP) {
+      edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
+      /* 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);
@@ -2781,10 +2786,13 @@ connection_handle_write_impl(connection_t *conn, int force)
     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);
-    /*XXXX022 check for overflow.*/
-    edge_conn->n_written += (int)n_written;
+    /* Check for overflow: */
+    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);