]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: quic: Avoid sending truncated datagrams
authorFrédéric Lécaille <flecaille@haproxy.com>
Wed, 3 Aug 2022 18:52:20 +0000 (20:52 +0200)
committerWilly Tarreau <w@1wt.eu>
Wed, 3 Aug 2022 19:09:04 +0000 (21:09 +0200)
There is a remaining loop in this ugly qc_snd_buf() function which could
lead haproxy to send truncated UDP datagrams. For now on, we send
a complete UDP datagram or nothing!

Must be backported to 2.6.

src/quic_sock.c

index 51d887a2cc2adb33424c6a26476e4e66b525b63e..12d94b71bf8198e1b04e93ef4d6b6b9c43b731e4 100644 (file)
@@ -362,64 +362,46 @@ void quic_sock_fd_iocb(int fd)
        MT_LIST_APPEND(&l->rx.rxbuf_list, &rxbuf->mt_list);
 }
 
-/* TODO standardize this function for a generic UDP sendto wrapper. This can be
+/* Send a datagram stored into <buf> buffer with <sz> as size.
+ * The caller must ensure there is at least <sz> bytes in this buffer.
+ * Return the size of this datagram if succeeded, 0 if truncated and -1 in case of
+ * any error.
+ * TODO standardize this function for a generic UDP sendto wrapper. This can be
  * done by removing the <qc> arg and replace it with address/port.
  */
-size_t qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t count,
+size_t qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz,
                   int flags)
 {
        ssize_t ret;
-       size_t try, done;
-       int send_flag;
 
-       done = 0;
-       /* send the largest possible block. For this we perform only one call
-        * to send() unless the buffer wraps and we exactly fill the first hunk,
-        * in which case we accept to do it once again.
-        */
-       while (count) {
-               try = b_contig_data(buf, done);
-               if (try > count)
-                       try = count;
-
-               send_flag = MSG_DONTWAIT | MSG_NOSIGNAL;
-               if (try < count || flags & CO_SFL_MSG_MORE)
-                       send_flag |= MSG_MORE;
-
-               ret = sendto(qc->li->rx.fd, b_peek(buf, done), try, send_flag,
+       do {
+               ret = sendto(qc->li->rx.fd, b_peek(buf, b_head_ofs(buf)), sz,
+                            MSG_DONTWAIT | MSG_NOSIGNAL,
                             (struct sockaddr *)&qc->peer_addr, get_addr_len(&qc->peer_addr));
-               if (ret > 0) {
-                       /* TODO remove partial sending support for UDP */
-                       count -= ret;
-                       done += ret;
+       } while (ret < 0 && errno == EINTR);
 
-                       if (ret < try)
-                               break;
-               }
-               else if (errno == EINTR) {
-                       /* try again */
-                       continue;
-               }
-               else if (ret == 0 || errno == EAGAIN || errno == EWOULDBLOCK || errno == ENOTCONN || errno == EINPROGRESS || errno == EBADF) {
-                       /* TODO must be handle properly. It is justified for UDP ? */
-                       qc->sendto_err++;
-                       break;
-               }
-               else if (errno) {
-                       /* TODO unlisted errno : handle it explicitely. */
-                       ABORT_NOW();
-               }
-       }
+       if (ret > 0) {
+               if (ret != sz)
+                       return 0;
 
-       if (done > 0) {
                /* we count the total bytes sent, and the send rate for 32-byte
                 * blocks. The reason for the latter is that freq_ctr are
                 * limited to 4GB and that it's not enough per second.
                 */
-               _HA_ATOMIC_ADD(&global.out_bytes, done);
-               update_freq_ctr(&global.out_32bps, (done + 16) / 32);
+               _HA_ATOMIC_ADD(&global.out_bytes, ret);
+               update_freq_ctr(&global.out_32bps, (ret + 16) / 32);
        }
-       return done;
+       else if (ret == 0 || errno == EAGAIN || errno == EWOULDBLOCK ||
+                errno == ENOTCONN || errno == EINPROGRESS || errno == EBADF) {
+               /* TODO must be handle properly. It is justified for UDP ? */
+               qc->sendto_err++;
+       }
+       else if (errno) {
+               /* TODO unlisted errno : handle it explicitely. */
+               ABORT_NOW();
+       }
+
+       return ret;
 }