]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: quic: Avoid trying to send ACK frames from an empty ack ranges tree
authorFrédéric Lécaille <flecaille@haproxy.com>
Tue, 7 Nov 2023 17:27:50 +0000 (18:27 +0100)
committerFrédéric Lécaille <flecaille@haproxy.com>
Thu, 9 Nov 2023 09:32:31 +0000 (10:32 +0100)
This may happen upon ack ranges allocation failures (from quic_update_ack_ranges_list().
This can lead to empty trees of ack ranges to be used to build ACK frames which
is not good at all. Furthermore this is detected by a BUG_ON() (in qc_do_build_pkt()).

To avoid this, simply update the acknowledgemen state of the connection only if
quic_update_ack_ranges_list() succeeds, as it fails only in case of memory
allocation failures.

Must be backported as far as 2.6.

src/quic_rx.c

index e2f3f8956330f73021212ba6560adf298aa2198d..37ab52176bccc34ee8ed61248c135edc303cf58f 100644 (file)
@@ -1381,23 +1381,27 @@ int qc_treat_rx_pkts(struct quic_conn *qc)
                                else {
                                        struct quic_arng ar = { .first = pkt->pn, .last = pkt->pn };
 
-                                       if (pkt->flags & QUIC_FL_RX_PACKET_ACK_ELICITING) {
-                                               int arm_ack_timer =
-                                                       qc->state >= QUIC_HS_ST_COMPLETE &&
-                                                       qel->pktns == qc->apktns;
-
-                                               qel->pktns->flags |= QUIC_FL_PKTNS_ACK_REQUIRED;
-                                               qel->pktns->rx.nb_aepkts_since_last_ack++;
-                                               qc_idle_timer_rearm(qc, 1, arm_ack_timer);
-                                       }
-                                       if (pkt->pn > largest_pn) {
-                                               largest_pn = pkt->pn;
-                                               largest_pn_time_received = pkt->time_received;
-                                       }
                                        /* Update the list of ranges to acknowledge. */
-                                       if (!quic_update_ack_ranges_list(qc, &qel->pktns->rx.arngs, &ar))
+                                       if (quic_update_ack_ranges_list(qc, &qel->pktns->rx.arngs, &ar)) {
+                                               if (pkt->flags & QUIC_FL_RX_PACKET_ACK_ELICITING) {
+                                                       int arm_ack_timer =
+                                                               qc->state >= QUIC_HS_ST_COMPLETE &&
+                                                               qel->pktns == qc->apktns;
+
+                                                       qel->pktns->flags |= QUIC_FL_PKTNS_ACK_REQUIRED;
+                                                       qel->pktns->rx.nb_aepkts_since_last_ack++;
+                                                       qc_idle_timer_rearm(qc, 1, arm_ack_timer);
+                                               }
+
+                                               if (pkt->pn > largest_pn) {
+                                                       largest_pn = pkt->pn;
+                                                       largest_pn_time_received = pkt->time_received;
+                                               }
+                                       }
+                                       else {
                                                TRACE_ERROR("Could not update ack range list",
                                                            QUIC_EV_CONN_RXPKT, qc);
+                                       }
                                }
                        }
                        node = eb64_next(node);