From 5b31989a3f3b417bc314d666a67d5d07a46205a4 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 19 Feb 2024 17:27:07 +0100 Subject: [PATCH] BUG/MEDIUM: quic: fix transient send error with listener socket Transient send errors is handled differentely if using connection or listener socket for QUIC transfers. In the first case, proper poller subscription is used via fd_cant_send()/fd_want_send(). For the listener socket case, error is ignored by qc_snd_buf() caller and retransmission mechanism will allow to reemit the data. For listener socket, transient error code handling is buggy. It blindly uses fd_cand_send() with member which is set to -1 for listener socket usage. This results in an invalid fdtab access, with a possible crash or a modification of a totally unrelated FD. This bug is simply fixed by using qc_test_fd() before using fd_cant_send()/fd_want_send(). This ensures is used only if initialized which is only the case when using connection socket. No crash was reported yet for this bug. However, it is reproducible by using ASAN compilation and the following strace sendmsg() errno command injection : # strace -qq -yy -p $(pgrep haproxy) -f -e trace=%network \ -e inject=sendto,sendmsg:error=EAGAIN:when=20+20 This must be backported up to 2.7. --- src/quic_sock.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/quic_sock.c b/src/quic_sock.c index c479249b7..f79651334 100644 --- a/src/quic_sock.c +++ b/src/quic_sock.c @@ -686,14 +686,16 @@ int qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz, if (ret < 0) { if (errno == EAGAIN || errno == EWOULDBLOCK || errno == ENOTCONN || errno == EINPROGRESS) { + /* transient error */ if (errno == EAGAIN || errno == EWOULDBLOCK) qc->cntrs.socket_full++; else qc->cntrs.sendto_err++; - /* transient error */ - fd_want_send(qc->fd); - fd_cant_send(qc->fd); + if (qc_test_fd(qc)) { + fd_want_send(qc->fd); + fd_cant_send(qc->fd); + } TRACE_PRINTF(TRACE_LEVEL_USER, QUIC_EV_CONN_SPPKTS, qc, 0, 0, 0, "UDP send failure errno=%d (%s)", errno, strerror(errno)); return 0;