[PATCH 1/2] seq_buf: fix overflow in seq_buf_putmem_hex()

[PATCH 1/2] seq_buf: fix overflow in seq_buf_putmem_hex()

[Yun Zhou]

There's two variables being increased in that loop (i and j), and i
follows the raw data, and j follows what is being written into the buffer.
We should compare 'i' to MAX_MEMHEX_BYTES or compare 'j' to HEX_CHARS.
Otherwise, if 'j' goes bigger than HEX_CHARS, it will overflow the
destination buffer.

This bug exists in the original code (commit 5e3ca0ec76fce 'ftrace:
introduce the "hex" output method'). Although its original design did
not support more than 8 bytes, the only check on length seems to have
mistaken the comparison object, 'len' should compare to 'HEX_CHARS/2'.
    BUG_ON(len >= HEX_CHARS);

Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 lib/seq_buf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 6aabb609dd87..223fbc3bb958 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -228,8 +228,10 @@ int seq_buf_putmem_hex(struct seq_buf *s, const void *mem,
 
 	WARN_ON(s->size == 0);
 
+	BUILD_BUG_ON(MAX_MEMHEX_BYTES * 2 >= HEX_CHARS);
+
 	while (len) {
-		start_len = min(len, HEX_CHARS - 1);
+		start_len = min(len, MAX_MEMHEX_BYTES);
 #ifdef __BIG_ENDIAN
 		for (i = 0, j = 0; i < start_len; i++) {
 #else
-- 
2.26.1

[PATCH 2/2] seq_buf: Make trace_seq_putmem_hex() support data longer than 8

[Yun Zhou]

Since the raw memory 'data' does not go forward, it will dump repeated
data if the data length is more than 8. If we want to dump longer data
blocks, we need to repeatedly call macro SEQ_PUT_HEX_FIELD. I think it
is a bit redundant, and multiple function calls also affect the performance.

This patch is to improve the commit 6d2289f3faa7 ("tracing: Make
trace_seq_putmem_hex() more robust").

Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 lib/seq_buf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 223fbc3bb958..562e53c93b7b 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -244,12 +244,14 @@ int seq_buf_putmem_hex(struct seq_buf *s, const void *mem,
 			break;
 
 		/* j increments twice per loop */
-		len -= j / 2;
 		hex[j++] = ' ';
 
 		seq_buf_putmem(s, hex, j);
 		if (seq_buf_has_overflowed(s))
 			return -1;
+
+		len -= start_len;
+		data += start_len;
 	}
 	return 0;
 }
-- 
2.26.1

[PATCH 1/2] seq_buf: fix overflow when length is bigger than 8

[Yun Zhou]

There's two variables being increased in that loop (i and j), and i
follows the raw data, and j follows what is being written into the buffer.
We should compare 'i' to MAX_MEMHEX_BYTES or compare 'j' to HEX_CHARS.
Otherwise, if 'j' goes bigger than HEX_CHARS, it will overflow the
destination buffer.

This bug was introduced by commit 6d2289f3faa71dcc ("tracing: Make
trace_seq_putmem_hex() more robust")

Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 lib/seq_buf.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 6aabb609dd87..aa2f666e584e 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -210,7 +210,8 @@ int seq_buf_putmem(struct seq_buf *s, const void *mem, unsigned int len)
  * seq_buf_putmem_hex - write raw memory into the buffer in ASCII hex
  * @s: seq_buf descriptor
  * @mem: The raw memory to write its hex ASCII representation of
- * @len: The length of the raw memory to copy (in bytes)
+ * @len: The length of the raw memory to copy (in bytes).
+ *       It can be not larger than 8.
  *
  * This is similar to seq_buf_putmem() except instead of just copying the
  * raw memory into the buffer it writes its ASCII representation of it
@@ -228,27 +229,19 @@ int seq_buf_putmem_hex(struct seq_buf *s, const void *mem,
 
 	WARN_ON(s->size == 0);
 
-	while (len) {
-		start_len = min(len, HEX_CHARS - 1);
+	start_len = min(len, MAX_MEMHEX_BYTES);
 #ifdef __BIG_ENDIAN
-		for (i = 0, j = 0; i < start_len; i++) {
+	for (i = 0, j = 0; i < start_len; i++) {
 #else
-		for (i = start_len-1, j = 0; i >= 0; i--) {
+	for (i = start_len-1, j = 0; i >= 0; i--) {
 #endif
-			hex[j++] = hex_asc_hi(data[i]);
-			hex[j++] = hex_asc_lo(data[i]);
-		}
-		if (WARN_ON_ONCE(j == 0 || j/2 > len))
-			break;
-
-		/* j increments twice per loop */
-		len -= j / 2;
-		hex[j++] = ' ';
-
-		seq_buf_putmem(s, hex, j);
-		if (seq_buf_has_overflowed(s))
-			return -1;
+		hex[j++] = hex_asc_hi(data[i]);
+		hex[j++] = hex_asc_lo(data[i]);
 	}
+
+	seq_buf_putmem(s, hex, j);
+	if (seq_buf_has_overflowed(s))
+		return -1;
 	return 0;
 }
 
-- 
2.26.1

[PATCH 2/2] seq_buf: Make trace_seq_putmem_hex() support data longer than 8

[Yun Zhou]

At present, trace_seq_putmem_hex() can only support data with length
of 8 or less, which greatly limits its application scope. If we want to
dump longer data blocks, we need to repeatedly call macro SEQ_PUT_HEX_FIELD.
I think it is a bit redundant, and multiple function calls also affect
the performance.

This patch is to perfect the commit 6d2289f3faa7 ("tracing: Make
trace_seq_putmem_hex() more robust").

Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 lib/seq_buf.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index aa2f666e584e..98580a5c32c0 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -210,8 +210,7 @@ int seq_buf_putmem(struct seq_buf *s, const void *mem, unsigned int len)
  * seq_buf_putmem_hex - write raw memory into the buffer in ASCII hex
  * @s: seq_buf descriptor
  * @mem: The raw memory to write its hex ASCII representation of
- * @len: The length of the raw memory to copy (in bytes).
- *       It can be not larger than 8.
+ * @len: The length of the raw memory to copy (in bytes)
  *
  * This is similar to seq_buf_putmem() except instead of just copying the
  * raw memory into the buffer it writes its ASCII representation of it
@@ -229,19 +228,27 @@ int seq_buf_putmem_hex(struct seq_buf *s, const void *mem,
 
 	WARN_ON(s->size == 0);
 
-	start_len = min(len, MAX_MEMHEX_BYTES);
+	while (len) {
+		start_len = min(len, MAX_MEMHEX_BYTES);
 #ifdef __BIG_ENDIAN
-	for (i = 0, j = 0; i < start_len; i++) {
+		for (i = 0, j = 0; i < start_len; i++) {
 #else
-	for (i = start_len-1, j = 0; i >= 0; i--) {
+		for (i = start_len-1, j = 0; i >= 0; i--) {
 #endif
-		hex[j++] = hex_asc_hi(data[i]);
-		hex[j++] = hex_asc_lo(data[i]);
-	}
+			hex[j++] = hex_asc_hi(data[i]);
+			hex[j++] = hex_asc_lo(data[i]);
+		}
 
-	seq_buf_putmem(s, hex, j);
-	if (seq_buf_has_overflowed(s))
-		return -1;
+		/* j increments twice per loop */
+		len -= j / 2;
+		hex[j++] = ' ';
+
+		seq_buf_putmem(s, hex, j);
+		if (seq_buf_has_overflowed(s))
+			return -1;
+
+		data += start_len;
+	}
 	return 0;
 }
 
-- 
2.26.1