From b7159f80b20432d510d43d9c8b57cf3f2b39159b Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Thu, 18 Feb 2016 23:56:29 +0900
Subject: [PATCH] Eliminate the possibility of nghttp2_stream.cycle overflow

---
 lib/nghttp2_stream.c | 42 +++++++++++++++++++++++++++++++-----------
 lib/nghttp2_stream.h |  4 ++--
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c
index 3e848d8e..70325bc3 100644
--- a/lib/nghttp2_stream.c
+++ b/lib/nghttp2_stream.c
@@ -30,14 +30,32 @@
 #include "nghttp2_session.h"
 #include "nghttp2_helper.h"
 
+/* Maximum distance between any two stream's cycle in the same
+   prirority queue.  Imagine stream A's cycle is A, and stream B's
+   cycle is B, and A < B.  The cycle is unsigned 32 bit integer, it
+   may get overflow.  Because of how we calculate the next cycle
+   value, if B - A is less than or equals to
+   NGHTTP2_MAX_CYCLE_DISTANCE, A and B are in the same scale, in other
+   words, B is really greater than or equal to A.  Otherwise, A is a
+   result of overflow, and it is actually A > B if we consider that
+   fact. */
+#define NGHTTP2_MAX_CYCLE_DISTANCE (16384 * 256 + 255)
+
 static int stream_less(const void *lhsx, const void *rhsx) {
   const nghttp2_stream *lhs, *rhs;
 
   lhs = nghttp2_struct_of(lhsx, nghttp2_stream, pq_entry);
   rhs = nghttp2_struct_of(rhsx, nghttp2_stream, pq_entry);
 
-  return lhs->cycle < rhs->cycle ||
-         (lhs->cycle == rhs->cycle && lhs->seq < rhs->seq);
+  if (lhs->cycle == rhs->cycle) {
+    return lhs->seq < rhs->seq;
+  }
+
+  if (lhs->cycle < rhs->cycle) {
+    return rhs->cycle - lhs->cycle <= NGHTTP2_MAX_CYCLE_DISTANCE;
+  }
+
+  return lhs->cycle - rhs->cycle > NGHTTP2_MAX_CYCLE_DISTANCE;
 }
 
 void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id,
@@ -116,14 +134,14 @@ static int stream_subtree_active(nghttp2_stream *stream) {
 /*
  * Returns next cycle for |stream|.
  */
-static void stream_next_cycle(nghttp2_stream *stream, uint64_t last_cycle) {
-  size_t penalty;
+static void stream_next_cycle(nghttp2_stream *stream, uint32_t last_cycle) {
+  uint32_t penalty;
 
-  penalty =
-      stream->last_writelen * NGHTTP2_MAX_WEIGHT + stream->pending_penalty;
+  penalty = (uint32_t)stream->last_writelen * NGHTTP2_MAX_WEIGHT +
+            stream->pending_penalty;
 
   stream->cycle = last_cycle + penalty / (uint32_t)stream->weight;
-  stream->pending_penalty = (uint32_t)(penalty % (uint32_t)stream->weight);
+  stream->pending_penalty = penalty % (uint32_t)stream->weight;
 }
 
 static int stream_obq_push(nghttp2_stream *dep_stream, nghttp2_stream *stream) {
@@ -229,9 +247,9 @@ void nghttp2_stream_reschedule(nghttp2_stream *stream) {
 
 void nghttp2_stream_change_weight(nghttp2_stream *stream, int32_t weight) {
   nghttp2_stream *dep_stream;
-  uint64_t last_cycle;
+  uint32_t last_cycle;
   int32_t old_weight;
-  size_t wlen_penalty;
+  uint32_t wlen_penalty;
 
   if (stream->weight == weight) {
     return;
@@ -254,7 +272,7 @@ void nghttp2_stream_change_weight(nghttp2_stream *stream, int32_t weight) {
 
   nghttp2_pq_remove(&dep_stream->obq, &stream->pq_entry);
 
-  wlen_penalty = stream->last_writelen * NGHTTP2_MAX_WEIGHT;
+  wlen_penalty = (uint32_t)stream->last_writelen * NGHTTP2_MAX_WEIGHT;
 
   /* Compute old stream->pending_penalty we used to calculate
      stream->cycle */
@@ -270,7 +288,9 @@ void nghttp2_stream_change_weight(nghttp2_stream *stream, int32_t weight) {
      place */
   stream_next_cycle(stream, last_cycle);
 
-  if (stream->cycle < dep_stream->descendant_last_cycle) {
+  if (stream->cycle < dep_stream->descendant_last_cycle &&
+      (dep_stream->descendant_last_cycle - stream->cycle) <=
+          NGHTTP2_MAX_CYCLE_DISTANCE) {
     stream->cycle = dep_stream->descendant_last_cycle;
   }
 
diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h
index b5d07592..da0e5d53 100644
--- a/lib/nghttp2_stream.h
+++ b/lib/nghttp2_stream.h
@@ -147,9 +147,9 @@ struct nghttp2_stream {
   /* Received body so far */
   int64_t recv_content_length;
   /* Base last_cycle for direct descendent streams. */
-  uint64_t descendant_last_cycle;
+  uint32_t descendant_last_cycle;
   /* Next scheduled time to sent item */
-  uint64_t cycle;
+  uint32_t cycle;
   /* Next seq used for direct descendant streams */
   uint64_t descendant_next_seq;
   /* Secondary key for prioritization to break a tie for cycle.  This