BUG/MINOR: htx: Perform raw copy for messages of same size in htx_copy_msg()

htx_copy_msg() takes a shortcut when the destination message is empty and copies
the whole source area, HTX header included, in one memcpy():

	if (htx_is_empty(htx) && htx_free_space(htx)) {
		memcpy(htx, msg->area, msg->size);
		return 1;
	}

but it never verifies that the destination buffer is at least as large as
<msg->size>. The only caller, http_reply_to_htx(), copies an error message
buffer, which http_str_to_htx() always allocates with a size of
global.tune.bufsize, into the response channel buffer. Two things follow:

  - if the destination were smaller, this would be a plain heap overflow. It is
    not reachable today because the response channel buffer is never a small
    one: only the request channel may be moved to a small buffer, by the
    PR_O2_USE_SBUF_QUEUE code in stream.c, and the L7-retry buffer is not a
    channel. But nothing states nor checks that invariant, and small buffers
    are recent, so this is a landmine.

  - if the destination is larger, which does happen since "http-response
    wait-for-body <time> use-large-buffer" moves the response channel to a large
    buffer, the copy also installs the source's ->size, so the destination HTX
    message ends up believing it is only bufsize-sized. That is harmless (it
    only under-uses the buffer and heals on the next reset) but wrong.

Let's restrict the raw copy to the case where both underlying buffers have
exactly the same size, and fall back to the existing block-per-block append
otherwise.

This should be backported to all supported versions.

diff --git a/include/haproxy/htx.h b/include/haproxy/htx.h
index f6181908cd5d52df4c7eed0a69a1edd202c7db1c..9e87357259a8d24064db085a1777659826e96db0 100644 (file)
--- a/include/haproxy/htx.h
+++ b/include/haproxy/htx.h
@@ -820,14 +820,18 @@ static inline int htx_set_eom(struct htx *htx)
  */
 static inline int htx_copy_msg(struct htx *htx, const struct buffer *msg)
 {
-       /* The destination HTX message is allocated and empty, we can do a raw copy */
-       if (htx_is_empty(htx) && htx_free_space(htx)) {
+       struct htx *htx_msg = htxbuf(msg);
+
+       /* The destination HTX message is empty and the message to append has
+        * has the same size, we can do a raw copy.
+        */
+       if (htx_is_empty(htx) && htx->size == htx_msg->size) {
                memcpy(htx, msg->area, msg->size);
                return 1;
        }
 
-       /* Otherwise, we need to append the HTX message */
-       return htx_append_msg(htx, htxbuf(msg));
+       /* Otherwise, we need to append the HTX message, block per block */
+       return htx_append_msg(htx, htx_msg);
 }
 
 /* Remove all blocks except headers. Trailers will also be removed too. */