[tor-commits] [tor/master] Fix some 'impossible' overflow bugs in byte counting

[nickm at torproject.org]

commit dddd333a80ee2e9bb731cb3c127ace3741d49673
Author: Nick Mathewson <nickm at torproject.org>
Date:   Fri Mar 25 16:45:25 2011 -0400

    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.
---
 changes/count_overflow |    5 +++++
 src/or/connection.c    |   36 ++++++++++++++++++++++--------------
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/changes/count_overflow b/changes/count_overflow
new file mode 100644
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 7fa6cd9..084237d 100644
--- 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);