BUG/MEDIUM: mux-quic: prevent BUG_ON() by refreshing frms on MAX_DATA
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 19 Dec 2024 13:17:37 +0000 (14:17 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 6 Jan 2025 10:27:04 +0000 (11:27 +0100)
commite48ee215fc253f9bc596342f50cb284afbcf642a
tree897823bc7b006105bf3fab7012f6ccf10c434d70
parentd534f28ac1d8bf418863796cbe58527466cd642c
BUG/MEDIUM: mux-quic: prevent BUG_ON() by refreshing frms on MAX_DATA

QUIC MUX emission has been optimized recently by recycling STREAM frames
list between emission cycles. This is done via qcc frms list member. If
new data is available, frames list must be cleared before the next
emission to force the encoding of new STREAM frames.

If a refresh frames list is missed, it would lead to incomplete data
emission on the next transfer. In most cases, this is detected via a
BUG_ON() inside qcc_io_send(), as qcs instances remains in send_list
after a qcc_send_frames() full emission.

A bug was recently found which causes this BUG_ON() crash. This is
directly related to flow control. Indeed, when sending credit is
increased on the connection or a stream, frames list should be cleared
as new larger STREAM frames could be encoded. This was already performed
on MAX_DATA/MAX_STREAM_DATA reception but only if flow-control limit was
unblocked. However this is not the proper condition and it may lead to
insufficient frames refresh and thus this BUG_ON() crash.

Fix this by adjusting the condition for frames refresh on flow control
credit increase. Now, frames list is cleared if real offset is not
blocked and soft offset was equal or greater to the previous limit.
Indeed, this is the only case in which frames refreshing is necessary as
it would result in bigger encoded STREAM frames.

This bug was detected on QUIC interop with go-x-net client. It can also
be reproduced, albeit not systematically, using the following command :
  $ ngtcp2-client -q --no-quic-dump --no-http-dump \
    --exit-on-all-streams-close --max-data 10 \
    127.0.0.1 20443 -n10 "http://127.0.0.1:20443/?s=10k"

This bug appeared with the following patch. As it is scheduled for 3.1
backporting, the current fix should be backported with it.
  14710b5e6bf76834343d58db22e00b72590b16fe
  MEDIUM/OPTIM: mux-quic: do not rebuild frms list on every send

(cherry picked from commit 7edb2ffae7e899af58a6b4fe546ac0665f5d3c9d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
src/mux_quic.c