BUG/MINOR: buffers/threads: always clear a buffer's head before releasing it

A small race exists in buffers with "show sess all". This one wants to show
some information grabbed from the buffer (especially in HTX mode). But the
thread owning this buffer might just be releasing its area, right after a
free() or munmap() call, resulting in a head that is not seen as empty yet
though the area was released. It may then be dereferenced by "show sess all"
causing a crash. Note that in practice it only happens in debug mode with
UAF enabled, but it's tricky enough to fix it right now.

This should be backported to stable versions which support threads and a
store barrier. It's worth noting that by performing the clearing first,
b_free() and b_drop() now become two exact equivalent.

diff --git a/doc/internals/buffer-api.txt b/doc/internals/buffer-api.txt
index 13881f7eb51e4f28e923f07f4fef4e55306132e1..abb5e9f769e27675f78113948d60218c57b7722a 100644 (file)
--- a/doc/internals/buffer-api.txt
+++ b/doc/internals/buffer-api.txt
@@ -568,10 +568,10 @@ b_alloc_fast        | buffer *buf      | allocates a buffer and assigns it to
                     |                  | even if some memory is available
 --------------------+------------------+---------------------------------------
 __b_drop            | buffer *buf      | releases <buf> which must be allocated
-                    | ret: void        |
+                    | ret: void        | and marks it empty
 --------------------+------------------+---------------------------------------
 b_drop              | buffer *buf      | releases <buf> only if it is allocated
-                    | ret: void        |
+                    | ret: void        | and marks it empty
 --------------------+------------------+---------------------------------------
 b_free              | buffer *buf      | releases <buf> only if it is allocated
                     | ret: void        | and marks it empty

diff --git a/include/common/buffer.h b/include/common/buffer.h
index 3d6c97ca629283874dee37f81981af8420b40953..34a3e95d2d613b088efd518eb73d2501f8f8c0e3 100644 (file)
--- a/include/common/buffer.h
+++ b/include/common/buffer.h
@@ -109,13 +109,22 @@ static inline struct buffer *b_alloc_fast(struct buffer *buf)
        return buf;
 }
 
-/* Releases buffer <buf> (no check of emptiness) */
+/* Releases buffer <buf> (no check of emptiness). The buffer's head is marked
+ * empty.
+ */
 static inline void __b_drop(struct buffer *buf)
 {
-       pool_free(pool_head_buffer, buf->area);
+       char *area = buf->area;
+
+       /* let's first clear the area to save an occasional "show sess all"
+        * glancing over our shoulder from getting a dangling pointer.
+        */
+       *buf = BUF_NULL;
+       __ha_barrier_store();
+       pool_free(pool_head_buffer, area);
 }
 
-/* Releases buffer <buf> if allocated. */
+/* Releases buffer <buf> if allocated, and marks it empty. */
 static inline void b_drop(struct buffer *buf)
 {
        if (buf->size)
@@ -126,7 +135,6 @@ static inline void b_drop(struct buffer *buf)
 static inline void b_free(struct buffer *buf)
 {
        b_drop(buf);
-       *buf = BUF_NULL;
 }
 
 /* Ensures that <buf> is allocated. If an allocation is needed, it ensures that