BUG/MEDIUM: buffers: Fix how input/output data are injected into buffers

The function buffer_contig_space is buggy and could lead to pernicious bugs
(never hitted until now, AFAIK). This function should return the number of bytes
that can be written into the buffer at once (without wrapping).

First, this function is used to inject input data (bi_putblk) and to inject
output data (bo_putblk and bo_inject). But there is no context. So it cannot
decide where contiguous space should placed. For input data, it should be after
bi_end(buf) (ie, buf->p + buf->i modulo wrapping calculation). For output data,
it should be after bo_end(buf) (ie, buf->p) and input data are assumed to not
exist (else there is no space at all).

Then, considering we need to inject input data, this function does not always
returns the right value. And when we need to inject output data, we must be sure
to have no input data at all (buf->i == 0), else the result can also be wrong
(but this is the caller responsibility, so everything should be fine here).

The buffer can be in 3 different states:

 1) no wrapping

              <---- o ----><----- i ----->
 +------------+------------+-------------+------------+
 |            |oooooooooooo|iiiiiiiiiiiii|xxxxxxxxxxxx|
 +------------+------------+-------------+------------+
                           ^             <contig_space>
                           p             ^            ^
			                 l            r

 2) input wrapping

 ...--->            <---- o ----><-------- i -------...
 +-----+------------+------------+--------------------+
 |iiiii|xxxxxxxxxxxx|oooooooooooo|iiiiiiiiiiiiiiiiiiii|
 +-----+------------+------------+--------------------+
       <contig_space>            ^
       ^            ^            p
       l            r

 3) output wrapping

 ...------ o ------><----- i ----->            <----...
 +------------------+-------------+------------+------+
 |oooooooooooooooooo|iiiiiiiiiiiii|xxxxxxxxxxxx|oooooo|
 +------------------+-------------+------------+------+
                    ^             <contig_space>
                    p             ^            ^
		                  l            r

buffer_contig_space returns (l - r). The cases 1 and 3 are correctly
handled. But for the second case, r is wrong. It points on the buffer's end
(buf->data + buf->size). It should be bo_end(buf) (ie, buf->p - buf->o).

To fix the bug, the function has been splitted. Now, bi_contig_space and
bo_contig_space should be used to know the contiguous space available to insert,
respectively, input data and output data. For bo_contig_space, input data are
assumed to not exist. And the right version is used, depending what we want to
do.

In addition, to clarify the buffer's API, buffer_realign does not return value
anymore. So it has the same API than buffer_slow_realign.

This patch can be backported in 1.7, 1.6 and 1.5.

include/common/buffer.h

@@ -156,6 +156,41 @@ static inline int bo_contig_data(const struct buffer *b)
 	return b->o;
 }
 
+/* Return the amount of bytes that can be written into the input area at once
+ * including reserved space which may be overwritten (this is the caller
+ * responsibility to know if the reserved space is protected or not).
+*/
+static inline int bi_contig_space(const struct buffer *b)
+{
+	const char *left, *right;
+
+	left  = bi_end(b);
+	right = bo_ptr(b);
+
+	if (left >= right)
+		right = b->data + b->size;
+
+	return (right - left);
+}
+
+/* Return the amount of bytes that can be written into the output area at once
+ * including reserved space which may be overwritten (this is the caller
+ * responsibility to know if the reserved space is protected or not). Input data
+ * are assumed to not exist.
+*/
+static inline int bo_contig_space(const struct buffer *b)
+{
+	const char *left, *right;
+
+	left  = bo_end(b);
+	right = bo_ptr(b);
+
+	if (left >= right)
+		right = b->data + b->size;
+
+	return (right - left);
+}
+
 /* Return the buffer's length in bytes by summing the input and the output */
 static inline int buffer_len(const struct buffer *buf)
 {
@@ -226,21 +261,6 @@ static inline int buffer_contig_area(const struct buffer *buf, const char *start
 	return count;
 }
 
-/* Return the amount of bytes that can be written into the buffer at once,
- * including reserved space which may be overwritten.
- */
-static inline int buffer_contig_space(const struct buffer *buf)
-{
-	const char *left, *right;
-
-	if (buf->data + buf->o <= buf->p)
-		right = buf->data + buf->size;
-	else
-		right = buf->p + buf->size - buf->o;
-
-	left = buffer_wrap_add(buf, buf->p + buf->i);
-	return right - left;
-}
 
 /* Returns the amount of byte that can be written starting from <p> into the
  * input buffer at once, including reserved space which may be overwritten.
@@ -340,17 +360,13 @@ static inline void bi_fast_delete(struct buffer *buf, int n)
 	buf->p += n;
 }
 
-/*
- * Tries to realign the given buffer, and returns how many bytes can be written
- * there at once without overwriting anything.
- */
-static inline int buffer_realign(struct buffer *buf)
+/* Tries to realign the given buffer. */
+static inline void buffer_realign(struct buffer *buf)
 {
 	if (!(buf->i | buf->o)) {
 		/* let's realign the buffer to optimize I/O */
 		buf->p = buf->data;
 	}
-	return buffer_contig_space(buf);
 }
 
 /* Schedule all remaining buffer data to be sent. ->o is not touched if it
@@ -402,7 +418,7 @@ static inline int bo_putblk(struct buffer *b, const char *blk, int len)
 	if (!len)
 		return 0;
 
-	half = buffer_contig_space(b);
+	half = bo_contig_space(b);
 	if (half > len)
 		half = len;
 

src/channel.c

@@ -91,8 +91,8 @@ int bo_inject(struct channel *chn, const char *msg, int len)
 		return -2;
 	}
 
-	max = buffer_realign(chn->buf);
-
+	buffer_realign(chn->buf);
+	max = bo_contig_space(chn->buf);
 	if (len > max)
 		return max;
 
@@ -166,7 +166,7 @@ int bi_putblk(struct channel *chn, const char *blk, int len)
 		return 0;
 
 	/* OK so the data fits in the buffer in one or two blocks */
-	max = buffer_contig_space(chn->buf);
+	max = bi_contig_space(chn->buf);
 	memcpy(bi_end(chn->buf), blk, MIN(len, max));
 	if (len > max)
 		memcpy(chn->buf->data, blk + max, len - max);