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)
committerWilly Tarreau <w@1wt.eu>
Thu, 14 Dec 2023 10:30:46 +0000 (11:30 +0100)
commit1d2d9311ac03d55f49249433e211aad0bf3fc18e
tree071e0a46c7baa37ee305c4d6fde39eb4384ad664
parent17fa5e6fc643eb2c09ccb0d794500746b3376c17
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.

(cherry picked from commit af297f19f6ff954b739cde3fa76f99f665b556bd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
src/mux_quic.c