Skip to content

Commit e9b78e3

Browse files
committed
BUG/MINOR: quic: fix padding issue on INITIAL retransmit
On loss detection timer expiration, qc_dgrams_retransmit() is used to reemit lost packets. Different code paths are present depending on the active encryption level. If Initial level is still initialized, retransmit is performed both for Initial and Handshake spaces, by first retrieving the list of lost frames for each of them. Prior to this patch, Handshake level was always registered for emission after Initial, even if it dit not have any frame to reemit. In this case, most of the time it would result in a datagram containing Initial reemitted frames packet coalesced with a Handshake packet consisting only of a PADDING frame. This is because padding is only added for the last registered QEL. For QUIC backend support, this may cause issues. This is because contrary to QUIC server side, Initial and Handshake levels keys are not derived simultaneously for a QUIC client. Thus, if the latter keys are unavailable, Handshake packet cannot be encoded in sending, leaving a single Initial packet. However, this is now too late to add PADDING. Thus the resulting datagram is invalid : this triggers the BUG_ON() assert failure located on qc_txb_store(). This patch fixes this by amending qc_dgrams_retransmit(). Now, Handshake level is only registered for emission if there is frame to retransmit, which implies that Handshake keys are already available. Thus, PADDING will now either be added at Initial or Handshake level as expected. Note that this issue should not be present on QUIC frontend, due to Initial and Handshake keys derivation almost simultaneously. However, this should still be backported up to 3.0.
1 parent 34d5bfd commit e9b78e3

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

src/quic_tx.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,15 +1008,15 @@ int qc_dgrams_retransmit(struct quic_conn *qc)
10081008

10091009
qc_prep_hdshk_fast_retrans(qc, &ifrms, &hfrms);
10101010
TRACE_DEVEL("Avail. ack eliciting frames", QUIC_EV_CONN_FRMLIST, qc, &ifrms);
1011-
TRACE_DEVEL("Avail. ack eliciting frames", QUIC_EV_CONN_FRMLIST, qc, &hfrms);
10121011
if (!LIST_ISEMPTY(&ifrms)) {
10131012
ipktns->tx.pto_probe = 1;
1014-
if (!LIST_ISEMPTY(&hfrms))
1015-
hpktns->tx.pto_probe = 1;
1016-
10171013
qel_register_send(&send_list, qc->iel, &ifrms);
1018-
if (qc->hel)
1014+
1015+
if (!LIST_ISEMPTY(&hfrms)) {
1016+
TRACE_DEVEL("Avail. ack eliciting frames", QUIC_EV_CONN_FRMLIST, qc, &hfrms);
1017+
hpktns->tx.pto_probe = 1;
10191018
qel_register_send(&send_list, qc->hel, &hfrms);
1019+
}
10201020

10211021
sret = qc_send(qc, 1, &send_list, 0);
10221022
qc_free_frm_list(qc, &ifrms);

0 commit comments

Comments
 (0)