]> git.ipfire.org Git - thirdparty/haproxy.git/commit
BUG/MEDIUM: mux-quic: report early error on stream
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 13 Dec 2023 15:28:28 +0000 (16:28 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 14 Dec 2023 10:15:46 +0000 (11:15 +0100)
commitaf297f19f6ff954b739cde3fa76f99f665b556bd
tree6b2fa4acf30611081026fd31de72e25242422250
parent682f73b4fa6d76aa0b5b743fe92777822884772d
BUG/MEDIUM: mux-quic: report early error on stream

On STOP_SENDING reception, an error is notified to the stream layer as
no more data can be responded. However, this is not done if the stream
instance is not allocated (already freed for example).

The issue occurs if STOP_SENDING is received and the stream instance is
instantiated after it. It happens if a STREAM frame is received after it
with H3 HEADERS, which is valid in QUIC protocol due to UDP packet
reordering. In this case, stream layer is never notified about the
underlying error. Instead, reponse buffers are silently purged by the
MUX in qmux_strm_snd_buf().

This is suboptimal as there is no point in exchanging data from the
server if it cannot be eventually transferred back to the client.
However, aside from this consideration, no other issue occured. However,
this is not the case with QUIC mux-to-mux implementation. Now, if
mux-to-mux is used, qmux_strm_snd_buf() is bypassed and response if
transferred via nego_ff/done_ff callbacks. However, these functions did
not checked if QCS is already locally closed. This causes a crash when
qcc_send_stream() is called via done_ff.

To fix this crash, there is several approach, one of them would be to
adjust nego_ff/done_ff QUIC callbacks. However, another method has been
chosen. Now stream layer is flagged on error just after its
instantiation if the stream is already locally closed. This ensures that
mux-to-mux won't try to emit data as se_nego_ff() check if the opposide
SD is not on error before continuing.

Note that an alternative solution could be to not instantiate at all
stream layer if QCS is already locally closed. This is the most optimal
solution as it reduce unnecessary allocations and task processing.
However, it's not easy to implement so the easier bug fix has been
chosen for the moment.

This patch is labelled as MEDIUM as it can change behavior of all QCS
instances, wheter mux-to-mux is used or not, and thus could reveal other
architecture issues.

This should fix latest crash occurence on github issue #2392.

It should be backported up to 2.6, until a necessary period of
observation.
src/mux_quic.c