Merge pull request h2o#463 from deweerdt/quicly_sentmap_update-failures

`quicly_sentmap_update` failing is a fatal error
maxsharabayko · Jun 15, 2021 · 4c460f4 · 4c460f4
2 parents f2d244f + 30d8024
commit 4c460f4
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 17 deletions.
diff --git a/include/quicly/loss.h b/include/quicly/loss.h
@@ -173,8 +173,8 @@ int quicly_loss_detect_loss(quicly_loss_t *r, int64_t now, uint32_t max_ack_dela
 /**
  * initializes the sentmap iterator, evicting the entries considered too old.
  */
-void quicly_loss_init_sentmap_iter(quicly_loss_t *loss, quicly_sentmap_iter_t *iter, int64_t now, uint32_t max_ack_delay,
-                                   int is_closing);
+int quicly_loss_init_sentmap_iter(quicly_loss_t *loss, quicly_sentmap_iter_t *iter, int64_t now, uint32_t max_ack_delay,
+                                  int is_closing);
 /**
  * Returns the timeout for sentmap entries. This timeout is also used as the duration of CLOSING / DRAINING state, and therefore be
  * longer than 3PTO. At the moment, the value is 4PTO.

diff --git a/lib/loss.c b/lib/loss.c
@@ -21,8 +21,8 @@
  */
 #include "quicly/loss.h"
 
-void quicly_loss_init_sentmap_iter(quicly_loss_t *loss, quicly_sentmap_iter_t *iter, int64_t now, uint32_t max_ack_delay,
-                                   int is_closing)
+int quicly_loss_init_sentmap_iter(quicly_loss_t *loss, quicly_sentmap_iter_t *iter, int64_t now, uint32_t max_ack_delay,
+                                  int is_closing)
 {
     quicly_sentmap_init_iter(&loss->sentmap, iter);
 
@@ -33,10 +33,13 @@ void quicly_loss_init_sentmap_iter(quicly_loss_t *loss, quicly_sentmap_iter_t *i
      * heavy loss; in such case, 32 is more than enough, yet small enough that the memory footprint does not matter. */
     const quicly_sent_packet_t *sent;
     while ((sent = quicly_sentmap_get(iter))->sent_at <= retire_before && sent->cc_bytes_in_flight == 0) {
+        int ret;
         if (!is_closing && loss->sentmap.num_packets < 32)
             break;
-        quicly_sentmap_update(&loss->sentmap, iter, QUICLY_SENTMAP_EVENT_EXPIRED);
+        if ((ret = quicly_sentmap_update(&loss->sentmap, iter, QUICLY_SENTMAP_EVENT_EXPIRED)) != 0)
+            return ret;
     }
+    return 0;
 }
 
 int quicly_loss_detect_loss(quicly_loss_t *loss, int64_t now, uint32_t max_ack_delay, int is_1rtt_only,
@@ -52,7 +55,8 @@ int quicly_loss_detect_loss(quicly_loss_t *loss, int64_t now, uint32_t max_ack_d
 
     loss->loss_time = INT64_MAX;
 
-    quicly_loss_init_sentmap_iter(loss, &iter, now, max_ack_delay, 0);
+    if ((ret = quicly_loss_init_sentmap_iter(loss, &iter, now, max_ack_delay, 0)) != 0)
+        return ret;
 
     /* Mark packets as lost if they are smaller than the largest_acked and outside either time-threshold or packet-threshold
      * windows. Once marked as lost, cc_bytes_in_flight becomes zero. */
@@ -79,7 +83,8 @@ int quicly_loss_detect_loss(quicly_loss_t *loss, int64_t now, uint32_t max_ack_d
     }
 
     if (!is_1rtt_only) {
-        quicly_loss_init_sentmap_iter(loss, &iter, now, max_ack_delay, 0);
+        if ((ret = quicly_loss_init_sentmap_iter(loss, &iter, now, max_ack_delay, 0)) != 0)
+            return ret;
         sent = quicly_sentmap_get(&iter);
     }
 

diff --git a/lib/quicly.c b/lib/quicly.c
@@ -974,8 +974,12 @@ void crypto_stream_receive(quicly_stream_t *stream, size_t off, const void *src,
             if (conn->crypto.handshake_properties.client.early_data_acceptance == PTLS_EARLY_DATA_REJECTED) {
                 dispose_cipher(&conn->application->cipher.egress.key);
                 conn->application->cipher.egress.key = (struct st_quicly_cipher_context_t){NULL};
-                discard_sentmap_by_epoch(
-                    conn, 1u << QUICLY_EPOCH_1RTT); /* retire all packets with ack_epoch == 3; they are all 0-RTT packets */
+                /* retire all packets with ack_epoch == 3; they are all 0-RTT packets */
+                int ret;
+                if ((ret = discard_sentmap_by_epoch(conn, 1u << QUICLY_EPOCH_1RTT)) != 0) {
+                    initiate_close(conn, ret, QUICLY_FRAME_TYPE_CRYPTO, NULL);
+                    goto Exit;
+                }
             }
         }
     }
@@ -3540,19 +3544,21 @@ int quicly_send_stream(quicly_stream_t *stream, quicly_send_context_t *s)
     return 0;
 }
 
-static inline void init_acks_iter(quicly_conn_t *conn, quicly_sentmap_iter_t *iter)
+static inline int init_acks_iter(quicly_conn_t *conn, quicly_sentmap_iter_t *iter)
 {
-    quicly_loss_init_sentmap_iter(&conn->egress.loss, iter, conn->stash.now, conn->super.remote.transport_params.max_ack_delay,
-                                  conn->super.state >= QUICLY_STATE_CLOSING);
+    return quicly_loss_init_sentmap_iter(&conn->egress.loss, iter, conn->stash.now,
+                                         conn->super.remote.transport_params.max_ack_delay,
+                                         conn->super.state >= QUICLY_STATE_CLOSING);
 }
 
 int discard_sentmap_by_epoch(quicly_conn_t *conn, unsigned ack_epochs)
 {
     quicly_sentmap_iter_t iter;
     const quicly_sent_packet_t *sent;
-    int ret = 0;
+    int ret;
 
-    init_acks_iter(conn, &iter);
+    if ((ret = init_acks_iter(conn, &iter)) != 0)
+        return ret;
 
     while ((sent = quicly_sentmap_get(&iter))->packet_number != UINT64_MAX) {
         if ((ack_epochs & (1u << sent->ack_epoch)) != 0) {
@@ -3575,7 +3581,8 @@ static int mark_frames_on_pto(quicly_conn_t *conn, uint8_t ack_epoch, size_t *by
     const quicly_sent_packet_t *sent;
     int ret;
 
-    init_acks_iter(conn, &iter);
+    if ((ret = init_acks_iter(conn, &iter)) != 0)
+        return ret;
 
     while ((sent = quicly_sentmap_get(&iter))->packet_number != UINT64_MAX) {
         if (sent->ack_epoch == ack_epoch && sent->frames_in_flight) {
@@ -4813,8 +4820,9 @@ static int handle_ack_frame(quicly_conn_t *conn, struct st_quicly_handle_payload
                  conn->egress.loss.sentmap.bytes_in_flight);
 
     /* loss-detection  */
-    quicly_loss_detect_loss(&conn->egress.loss, conn->stash.now, conn->super.remote.transport_params.max_ack_delay,
-                            conn->initial == NULL && conn->handshake == NULL, on_loss_detected);
+    if ((ret = quicly_loss_detect_loss(&conn->egress.loss, conn->stash.now, conn->super.remote.transport_params.max_ack_delay,
+                                       conn->initial == NULL && conn->handshake == NULL, on_loss_detected)) != 0)
+        return ret;
     update_loss_alarm(conn, 0);
 
     return 0;