Skip to content

Commit

Permalink
BUG/MINOR: mux-quic: remove full demux flag on ncbuf release
Browse files Browse the repository at this point in the history
When rcv_buf stream callback is invoked, mux tasklet is woken up if
demux was previously blocked due to lack of buffer space. A BUG_ON() is
present to ensure there is data in qcs Rx buffer. If this is not the
case, wakeup is unneeded :

  BUG_ON(!ncb_data(&qcs->rx.ncbuf, 0));

This BUG_ON() may be triggered if RESET_STREAM is received after demux
has been blocked. On reset, Rx buffer is purged according to RFC 9000
which allows to discard any data not yet consumed. This will trigger the
BUG_ON() assertion if rcv_buf stream callback is invoked after this.

To prevent BUG_ON() crash, just clear demux block flag each time Rx
buffer is purged. This covers accordingly RESET_STREAM reception.

This should be backported up to 2.7.

This may fix github issue haproxy#2293.

This bug relies on several precondition so its occurence is rare. This
was reproduced by using a custom client which post big enough data to
fill the buffer. It then emits a RESET_STREAM in place of a proper FIN.
Moreover, mux code has been edited to artificially stalled stream read
to force demux blocking.

h3_data_to_htx:
-       return htx_sent;
+       return 1;

qcc_recv_reset_stream:
        qcs_free_ncbuf(qcs, &qcs->rx.ncbuf);
+       qcs_notify_recv(qcs);

qmux_strm_rcv_buf:
        char fin = 0;
+       static int i = 0;
+       if (++i < 2)
+               return 0;
        TRACE_ENTER(QMUX_EV_STRM_RECV, qcc->conn, qcs);

(cherry picked from commit 7cf9cf7)
Signed-off-by: Christopher Faulet <[email protected]>
  • Loading branch information
a-denoyelle authored and capflam committed Oct 4, 2023
1 parent 4bc0909 commit 217b0f4
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/mux_quic.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ static void qcs_free_ncbuf(struct qcs *qcs, struct ncbuf *ncbuf)
offer_buffers(NULL, 1);

*ncbuf = NCBUF_NULL;

/* Reset DEM_FULL as buffer is released. This ensures mux is not woken
* up from rcv_buf stream callback when demux was previously blocked.
*/
qcs->flags &= ~QC_SF_DEM_FULL;
}

/* Free <qcs> instance. This function is reserved for internal usage : it must
Expand Down Expand Up @@ -2743,9 +2748,8 @@ static size_t qmux_strm_rcv_buf(struct stconn *sc, struct buffer *buf,

/* Restart demux if it was interrupted on full buffer. */
if (ret && qcs->flags & QC_SF_DEM_FULL) {
/* There must be data left for demux if it was interrupted on
* full buffer. If this assumption is incorrect wakeup is not
* necessary.
/* Ensure DEM_FULL is only set if there is available data to
* ensure we never do unnecessary wakeup here.
*/
BUG_ON(!ncb_data(&qcs->rx.ncbuf, 0));

Expand Down

0 comments on commit 217b0f4

Please sign in to comment.