BUG/MEDIUM: htx: Fix a possible null derefs in htx_xfer_blks()

In htx_xfer_blks() function, when headers or trailers are partially
transferred, we rollback the copy by removing copied blocks. Internally, all
blocks between <dstref> and <dstblk> are removed. But if the transfer was
stopped because we failed to reserve a block, the variable <dstblk> is
NULL. Thus, we must not try to remove it. It is unexpected to call
htx_remove_blk() in this case.

htx_remove_blk() was updated to test <blk> variable inside the existing
BUG_ON(). The block must be defined.

For now, this bug may only be encountered when H2 trailers are copied. On H2
headers, the destination buffer is empty. Thus a swap is performed.

This patch should fix the issue #1578. It must be backported as far as 2.4.

diff --git a/src/htx.c b/src/htx.c
index 3cc1a34b05bf9d7598176a6ea2534720d9abe8a2..7e5bd46186ffb717e8fd906272e4ea8631fd755d 100644 (file)
--- a/src/htx.c
+++ b/src/htx.c
@@ -336,7 +336,7 @@ struct htx_blk *htx_remove_blk(struct htx *htx, struct htx_blk *blk)
        enum htx_blk_type type;
        uint32_t pos, addr, sz;
 
-       BUG_ON(htx->head == -1);
+       BUG_ON(!blk || htx->head == -1);
 
        /* This is the last block in use */
        if (htx->head == htx->tail) {
@@ -739,12 +739,14 @@ struct htx_ret htx_xfer_blks(struct htx *dst, struct htx *src, uint32_t count,
        }
 
        if (unlikely(dstref)) {
-               /* Headers or trailers part was partially xferred, so rollback the copy
-                * by removing all block between <dstref> and <dstblk>, both included.
+               /* Headers or trailers part was partially xferred, so rollback
+                * the copy by removing all block between <dstref> and <dstblk>,
+                * both included. <dstblk> may be NULL.
                 */
                while (dstref && dstref != dstblk)
                        dstref = htx_remove_blk(dst, dstref);
-               htx_remove_blk(dst, dstblk);
+               if (dstblk)
+                       htx_remove_blk(dst, dstblk);
 
                /* <dst> HTX message is empty, it means the headers or trailers
                 * part is too big to be copied at once.