]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: mux-h2: Count copied data when looping on RX bufs in h2_rcv_buf()
authorChristopher Faulet <cfaulet@haproxy.com>
Thu, 2 Jan 2025 08:39:06 +0000 (09:39 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Thu, 2 Jan 2025 08:58:23 +0000 (09:58 +0100)
When data was copied from RX buffers to the channel buffer, more data than
expected could be moved because amount of data copied was never decremented
from the limit. This could lead to a stream dead lock when the compression
filter was inuse.

The issue was introduced by commit 4eb3ff1 ("MAJOR: mux-h2: make streams use
the connection's buffers") but revealed by 3816c38 ("MAJOR: mux-h2: permit a
stream to allocate as many buffers as desired").

Because a h2 stream can now have several RX buffers, in h2_rcv_buf(), we
loop on these buffers to fill the channel buffer. However, we must still
take care to respect the limit to not copy to much data. However, the
"count" variable was never decremented to reflect amount of data already
copied. So, it was possible to exceed the limit.

It was an issue when the compression filter was inuse because the channel
buffer could be fully filled, preventing the compression to be
performed. When this happened, the stream was infinitly blocked because the
compression filter was asking for some space but nothing was scheduled to be
forwarded.

This patch should fix the issue #2826. It must be backported to 3.1.

src/mux_h2.c

index 03f4bfdfe08c4792a3cb2396db41f487d4b64f5a..bea96271bad61e148ffa2c241aeda6271a910f32 100644 (file)
@@ -7444,6 +7444,7 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in
        struct htx *h2s_htx = NULL;
        struct htx *buf_htx = NULL;
        struct buffer *rxbuf = NULL;
+       struct htx_ret htxret;
        size_t ret = 0;
        uint prev_h2c_flags = h2c->flags;
 
@@ -7470,13 +7471,15 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in
        /* <buf> is empty and the message is small enough, swap the
         * buffers. */
        if (htx_is_empty(buf_htx) && htx_used_space(h2s_htx) <= count) {
+               count -= h2s_htx->data;
                htx_to_buf(buf_htx, buf);
                htx_to_buf(h2s_htx, rxbuf);
                b_xfer(buf, rxbuf, b_data(rxbuf));
                goto end;
        }
 
-       htx_xfer_blks(buf_htx, h2s_htx, count, HTX_BLK_UNUSED);
+       htxret = htx_xfer_blks(buf_htx, h2s_htx, count, HTX_BLK_UNUSED);
+       count -= htxret.ret;
 
        if (h2s_htx->flags & HTX_FL_PARSING_ERROR) {
                buf_htx->flags |= HTX_FL_PARSING_ERROR;
@@ -7504,7 +7507,9 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in
        if (rxbuf && !b_size(rxbuf)) {
                h2s_put_rxbuf(h2s);
                h2c->flags &= ~(H2_CF_DEM_RXBUF | H2_CF_DEM_SFULL);
-               goto xfer_next_buf;
+               /* Copied more data if possible */
+               if (count)
+                       goto xfer_next_buf;
        }
 
        /* tell the stream layer whether there are data left or not */