kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] seq_buf: fix overflow in seq_buf_putmem_hex()
@ 2021-06-26  3:21 Yun Zhou
  2021-06-26  3:21 ` [PATCH 2/2] seq_buf: Make trace_seq_putmem_hex() support data longer than 8 Yun Zhou
  0 siblings, 1 reply; 3+ messages in thread
From: Yun Zhou @ 2021-06-26  3:21 UTC (permalink / raw)
  To: rostedt; +Cc: linux-kernel, kernel-hardening, ying.xue, zhiquan.li

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] seq_buf: Make trace_seq_putmem_hex() support data longer than 8
  2021-06-26  3:21 [PATCH 1/2] seq_buf: fix overflow in seq_buf_putmem_hex() Yun Zhou
@ 2021-06-26  3:21 ` Yun Zhou
  0 siblings, 0 replies; 3+ messages in thread
From: Yun Zhou @ 2021-06-26  3:21 UTC (permalink / raw)
  To: rostedt; +Cc: linux-kernel, kernel-hardening, ying.xue, zhiquan.li

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] seq_buf: Make trace_seq_putmem_hex() support data longer than 8
  2021-06-25 15:53 [PATCH 1/2] seq_buf: fix overflow when length is bigger " Yun Zhou
@ 2021-06-25 15:53 ` Yun Zhou
  0 siblings, 0 replies; 3+ messages in thread
From: Yun Zhou @ 2021-06-25 15:53 UTC (permalink / raw)
  To: rostedt; +Cc: linux-kernel, kernel-hardening, ying.xue, zhiquan.li

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-26  3:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-26  3:21 [PATCH 1/2] seq_buf: fix overflow in seq_buf_putmem_hex() Yun Zhou
2021-06-26  3:21 ` [PATCH 2/2] seq_buf: Make trace_seq_putmem_hex() support data longer than 8 Yun Zhou
  -- strict thread matches above, loose matches on Subject: below --
2021-06-25 15:53 [PATCH 1/2] seq_buf: fix overflow when length is bigger " Yun Zhou
2021-06-25 15:53 ` [PATCH 2/2] seq_buf: Make trace_seq_putmem_hex() support data longer " Yun Zhou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).