BUG/MEDIUM: mux-h2: Reset MUX blocking flags when a send error is caught

When an send error is detected on the underlying connection, a pending error
is reported to the H2 connection by setting H2_CF_ERR_PENDING flag. When
this happen the tail of the mux ring buffer is reset. However some blocking
flags remain set and have no chance to be removed later because of the
pending error. Especially the flag H2_CF_DEM_MROOM which block data
demultiplexing. Thus, it is possible to block a H2 connection with unparsed
incoming data.

Worse, if a read event is received, it could lead to a wakeup loop between
the H2 connection and the underlying SSL connection. The H2 connection is
unable to convert the pending error to a fatal error because the
demultiplexing is blocked. In the mean time, it tries to receive more data
because of the not-consumed read event. On the underlying connection side,
the error detected earlier blocks the read, but the H2 connection is woken
up to handle the error.

To fix the issue, blocking flags must be removed when a send error is caught,
H2_CF_MUX_MFULL and H2_CF_DEM_MROOM flags. But, it is not necessary to only
release the tail of the mbuf ring. When a send error is detected, all outgoing
data can be flushed. So, now, in h2_send(), h2_release_mbuf() function is called
on pending error. The mbuf ring is fully released and H2_CF_MUX_MFULL and
H2_CF_DEM_MROOM flags are removed.

Many thanks to Krzysztof Kozłowski for its help to spot this issue.

This patch could be backported at least as far as 2.8. But it is a bit
sensitive. So, it is probably a good idea to backport it to 3.2 for now and
wait for bug report on older versions.
This commit is contained in:
Christopher Faulet 2025-09-09 07:55:01 +02:00
parent 0b6908385e
commit 626d7934cf

View File

@ -4800,7 +4800,7 @@ static int h2_send(struct h2c *h2c)
TRACE_DEVEL("leaving on error", H2_EV_H2C_SEND, h2c->conn); TRACE_DEVEL("leaving on error", H2_EV_H2C_SEND, h2c->conn);
if (h2c->flags & H2_CF_END_REACHED) if (h2c->flags & H2_CF_END_REACHED)
h2c->flags |= H2_CF_ERROR; h2c->flags |= H2_CF_ERROR;
b_reset(br_tail(h2c->mbuf)); h2_release_mbuf(h2c);
h2c->idle_start = now_ms; h2c->idle_start = now_ms;
return 1; return 1;
} }
@ -4899,7 +4899,7 @@ static int h2_send(struct h2c *h2c)
h2c_report_term_evt(h2c, muxc_tevt_type_snd_err); h2c_report_term_evt(h2c, muxc_tevt_type_snd_err);
if (h2c->flags & H2_CF_END_REACHED) if (h2c->flags & H2_CF_END_REACHED)
h2c->flags |= H2_CF_ERROR; h2c->flags |= H2_CF_ERROR;
b_reset(br_tail(h2c->mbuf)); h2_release_mbuf(h2c);
} }
/* We're not full anymore, so we can wake any task that are waiting /* We're not full anymore, so we can wake any task that are waiting