[PATCH] kdb: Fix buffer overflow during tab-complete

From: Daniel Thompson
Date: Mon Apr 15 2024 - 09:35:06 EST


Currently, when the user attempts symbol completion with the Tab key, kdb
will use strncpy() to insert the completed symbol into the command buffer.
Unfortunately it passes the size of the source buffer rather than the
destination to strncpy() with predictably horrible results. Most obviously
if the command buffer is already full but cp, the cursor position, is in
the middle of the buffer, then we will write past the end of the supplied
buffer.

Even when the buffer does not overflow there are other problems when cp is
in the middle of a line. For example the cursor position is not updated
correctly meaning what is shown to the user on the console is not what is
actually present in the command buffer.

Fix this by replacing the dubious strncpy() calls with memmove()/memcpy()
calls plus explicit boundary checks to make sure we have enough space
before we start moving characters around. We also add a second call to
kdb_printf() to fix the console cursor position if needed.

Reported-by: Justin Stitt <justinstitt@xxxxxxxxxx>
Signed-off-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
---
kernel/debug/kdb/kdb_io.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 9443bc63c5a24..3a1b9dd890f45 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -367,14 +367,23 @@ static char *kdb_read(char *buffer, size_t bufsize)
kdb_printf(kdb_prompt_str);
kdb_printf("%s", buffer);
} else if (tab != 2 && count > 0) {
- len_tmp = strlen(p_tmp);
- strncpy(p_tmp+len_tmp, cp, lastchar-cp+1);
- len_tmp = strlen(p_tmp);
- strncpy(cp, p_tmp+len, len_tmp-len + 1);
- len = len_tmp - len;
- kdb_printf("%s", cp);
- cp += len;
- lastchar += len;
+ /* How many new characters do we want from tmpbuffer? */
+ len_tmp = strlen(p_tmp) - len;
+ if (lastchar + len_tmp >= bufend)
+ len_tmp = bufend - lastchar - 1;
+
+ if (len_tmp) {
+ /* + 1 ensures the '\0' is memmove'd */
+ memmove(cp+len_tmp, cp, (lastchar-cp) + 1);
+ memcpy(cp, p_tmp+len, len_tmp);
+ lastchar += len_tmp;
+
+ kdb_printf("%s", cp);
+ cp += len_tmp;
+ if (cp != lastchar)
+ kdb_printf("\r%s%.*s", kdb_prompt_str,
+ (int)(cp - buffer), buffer);
+ }
}
kdb_nextline = 1; /* reset output line number */
break;
--
2.43.0