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.
(cherry picked from commit 234a10aa9bce34781b5b0b4c4ffb595e7563eb41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 14a874996379de6d9b72a24bc5f96382e9417dd3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/htx.c b/src/htx.c
index 588aa6c..e5af410 100644
--- a/src/htx.c
+++ b/src/htx.c
@@ -336,7 +336,7 @@
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) {
@@ -736,12 +736,14 @@
}
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.