mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2026-02-15 06:11:10 +01:00
Code Issues Projects Releases Wiki Activity

BUG/MEDIUM: quic: fix crash on MUX send notification

Browse Source 
MUX notification on TX has been edited recently : it will be notified
only when sending its own data, and not for example on retransmission by
the quic-conn layer. This is subject of the patch :
  b29a1dc2f4a334c1c7fea76c59abb4097422c05c
  BUG/MINOR: quic: do not notify MUX on frame retransmit

A new flag QUIC_FL_CONN_RETRANS_LOST_DATA has been introduced to
differentiate qc_send_app_pkts invocation by MUX and directly by the
quic-conn layer in quic_conn_app_io_cb(). However, this is a first
problem as internal quic-conn layer usage is not limited to
retransmission. For example for NEW_CONNECTION_ID emission.

Another problem much important is that send functions are also called
through quic_conn_io_cb() which has not been protected from MUX
notification. This could probably result in crash when trying to notify
the MUX.

To fix both problems, quic-conn flagging has been inverted : when used
by the MUX, quic-conn is flagged with QUIC_FL_CONN_TX_MUX_CONTEXT. To
improve the API, MUX must now used qc_send_mux which ensure the flag is
set. qc_send_app_pkts is now static and can only be used by the
quic-conn layer.

This must be backported wherever the previously mentionned patch is.
This commit is contained in:
Amaury Denoyelle 2022-08-17 16:33:13 +02:00
parent 4173a39c1f
commit 704675656b
4 changed files with 30 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/haproxy/xprt_quic-t.h
View File

@ -592,7 +592,7 @@ enum qc_mux_state {
#define QUIC_FL_CONN_POST_HANDSHAKE_FRAMES_BUILT (1U << 2)
#define QUIC_FL_CONN_LISTENER (1U << 3)
#define QUIC_FL_CONN_ACCEPT_REGISTERED (1U << 4)
#define QUIC_FL_CONN_RETRANS_LOST_DATA (1U << 5) /* retransmission in progress for lost data */
#define QUIC_FL_CONN_TX_MUX_CONTEXT (1U << 5) /* sending in progress from the MUX layer */
#define QUIC_FL_CONN_IDLE_TIMER_RESTARTED_AFTER_READ (1U << 6)
#define QUIC_FL_CONN_RETRANS_NEEDED (1U << 7)
#define QUIC_FL_CONN_RETRANS_OLD_DATA (1U << 8) /* retransmission in progress for probing with already sent data */

2
include/haproxy/xprt_quic.h
View File

@ -769,7 +769,7 @@ int quic_set_app_ops(struct quic_conn *qc, const unsigned char *alpn, size_t alp
struct task *quic_lstnr_dghdlr(struct task *t, void *ctx, unsigned int state);
int quic_get_dgram_dcid(unsigned char *buf, const unsigned char *end,
unsigned char **dcid, size_t *dcid_len);
int qc_send_app_pkts(struct quic_conn *qc, struct list *frms);
int qc_send_mux(struct quic_conn *qc, struct list *frms);
void qc_notify_close(struct quic_conn *qc);

2
src/mux_quic.c
View File

@ -1460,7 +1460,7 @@ static int qc_send_frames(struct qcc *qcc, struct list *frms)
LIST_INIT(&qcc->send_retry_list);
qc_send_app_pkts(qcc->conn->handle.qc, frms);
qc_send_mux(qcc->conn->handle.qc, frms);
/* If there is frames left at this stage, transport layer is blocked.
* Subscribe on it to retry later.

58
src/xprt_quic.c
View File

@ -3885,9 +3885,7 @@ static int qc_qel_may_rm_hp(struct quic_conn *qc, struct quic_enc_level *qel)
/* Try to send application frames from list <frms> on connection <qc>.
*
* For retransmission you must use wrapper depending on the sending condition :
* - use qc_send_app_retransmit to send data detected as lost
* - use qc_send_app_probing when probing with already sent data
* Use qc_send_app_probing wrapper when probing with old data.
*
* Returns 1 on success. Some data might not have been sent due to congestion,
* in this case they are left in <frms> input list. The caller may subscribe on
@ -3897,7 +3895,7 @@ static int qc_qel_may_rm_hp(struct quic_conn *qc, struct quic_enc_level *qel)
* TODO review and classify more distinctly transient from definitive errors to
* allow callers to properly handle it.
*/
int qc_send_app_pkts(struct quic_conn *qc, struct list *frms)
static int qc_send_app_pkts(struct quic_conn *qc, struct list *frms)
{
int status = 0;
struct buffer *buf;
@ -3943,27 +3941,6 @@ int qc_send_app_pkts(struct quic_conn *qc, struct list *frms)
goto leave;
}
/* Try to send application frames from list <frms> on connection <qc>. Use this
* function when retransmitting lost frames.
*
* Returns the result from qc_send_app_pkts function.
*/
static forceinline int qc_send_app_retransmit(struct quic_conn *qc,
struct list *frms)
{
int ret;
TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
TRACE_STATE("preparing lost data (retransmission)", QUIC_EV_CONN_TXPKT, qc);
qc->flags |= QUIC_FL_CONN_RETRANS_LOST_DATA;
ret = qc_send_app_pkts(qc, frms);
qc->flags &= ~QUIC_FL_CONN_RETRANS_LOST_DATA;
TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc);
return ret;
}
/* Try to send application frames from list <frms> on connection <qc>. Use this
* function when probing is required.
*
@ -3985,6 +3962,27 @@ static forceinline int qc_send_app_probing(struct quic_conn *qc,
return ret;
}
/* Try to send application frames from list <frms> on connection <qc>. This
* function is provided for MUX upper layer usage only.
*
* Returns the result from qc_send_app_pkts function.
*/
int qc_send_mux(struct quic_conn *qc, struct list *frms)
{
int ret;
TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
BUG_ON(qc->mux_state != QC_MUX_READY); /* Only MUX can uses this function so it must be ready. */
TRACE_STATE("preparing data (from MUX)", QUIC_EV_CONN_TXPKT, qc);
qc->flags |= QUIC_FL_CONN_TX_MUX_CONTEXT;
ret = qc_send_app_pkts(qc, frms);
qc->flags &= ~QUIC_FL_CONN_TX_MUX_CONTEXT;
TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc);
return ret;
}
/* Sends handshake packets from up to two encryption levels <tel> and <next_te>
* with <tel_frms> and <next_tel_frms> as frame list respectively for <qc>
* QUIC connection. <old_data> is used as boolean to send data already sent but
@ -4155,8 +4153,8 @@ static struct task *quic_conn_app_io_cb(struct task *t, void *context, unsigned
}
/* XXX TODO: how to limit the list frames to send */
if (!qc_send_app_retransmit(qc, &qel->pktns->tx.frms)) {
TRACE_DEVEL("qc_send_app_retransmit() failed", QUIC_EV_CONN_IO_CB, qc);
if (!qc_send_app_pkts(qc, &qel->pktns->tx.frms)) {
TRACE_DEVEL("qc_send_app_pkts() failed", QUIC_EV_CONN_IO_CB, qc);
goto out;
}
@ -6487,8 +6485,7 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
LIST_APPEND(outlist, &cf->list);
/* Do not notify MUX on retransmission. */
if (!(qc->flags & (QUIC_FL_CONN_RETRANS_LOST_DATA|QUIC_FL_CONN_RETRANS_OLD_DATA))) {
BUG_ON(qc->mux_state != QC_MUX_READY); /* MUX must be the caller if not on retransmission. */
if (qc->flags & QUIC_FL_CONN_TX_MUX_CONTEXT) {
qcc_streams_sent_done(cf->stream.stream->ctx,
cf->stream.len,
cf->stream.offset.key);
@ -6527,8 +6524,7 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
cf->stream.data = (unsigned char *)b_peek(&cf_buf, dlen);
/* Do not notify MUX on retransmission. */
if (!(qc->flags & (QUIC_FL_CONN_RETRANS_LOST_DATA|QUIC_FL_CONN_RETRANS_OLD_DATA))) {
BUG_ON(qc->mux_state != QC_MUX_READY); /* MUX must be the caller if not on retransmission. */
if (qc->flags & QUIC_FL_CONN_TX_MUX_CONTEXT) {
qcc_streams_sent_done(new_cf->stream.stream->ctx,
new_cf->stream.len,
new_cf->stream.offset.key);