Commit a7605628 authored by Linus Torvalds's avatar Linus Torvalds
Browse files
Pull kgdb updates from Daniel Thompson:
 "Nine patches this cycle and they split into just three topics:

   - Adopt coccinelle's recommendation to adopt str_plural()

   - A set of seven patches to refactor kdb_read() to improve both code
     clarity and its discipline with respect to fixed size buffers.

     This isn't just a refactor. Between them these also fix a cursor
     movement redraw problem and two buffer overflows (one latent and
     one real, albeit difficult to tickle).

   - Fix an NMI-safety problem when enqueuing kdb's keyboard reset code

  I wrote eight of the nine patches in this collection so many thanks to
  Doug Anderson for the reviews. The changes that affects
  drivers/tty/serial is acked by Greg KH"

* tag 'kgdb-6.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux:
  serial: kgdboc: Fix NMI-safety problems from keyboard reset code
  kdb: Simplify management of tmpbuffer in kdb_read()
  kdb: Replace double memcpy() with memmove() in kdb_read()
  kdb: Use format-specifiers rather than memset() for padding in kdb_read()
  kdb: Merge identical case statements in kdb_read()
  kdb: Fix console handling when editing and tab-completing commands
  kdb: Use format-strings rather than '\0' injection in kdb_read()
  kdb: Fix buffer overflow during tab-complete
  kdb: Use str_plural() to fix Coccinelle warning
parents 41c14f1a b2aba15a
Loading
Loading
Loading
Loading
+29 −1
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@
#include <linux/console.h>
#include <linux/vt_kern.h>
#include <linux/input.h>
#include <linux/irq_work.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/serial_core.h>
@@ -48,6 +49,25 @@ static struct kgdb_io kgdboc_earlycon_io_ops;
static int                      (*earlycon_orig_exit)(struct console *con);
#endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */

/*
 * When we leave the debug trap handler we need to reset the keyboard status
 * (since the original keyboard state gets partially clobbered by kdb use of
 * the keyboard).
 *
 * The path to deliver the reset is somewhat circuitous.
 *
 * To deliver the reset we register an input handler, reset the keyboard and
 * then deregister the input handler. However, to get this done right, we do
 * have to carefully manage the calling context because we can only register
 * input handlers from task context.
 *
 * In particular we need to trigger the action from the debug trap handler with
 * all its NMI and/or NMI-like oddities. To solve this the kgdboc trap exit code
 * (the "post_exception" callback) uses irq_work_queue(), which is NMI-safe, to
 * schedule a callback from a hardirq context. From there we have to defer the
 * work again, this time using schedule_work(), to get a callback using the
 * system workqueue, which runs in task context.
 */
#ifdef CONFIG_KDB_KEYBOARD
static int kgdboc_reset_connect(struct input_handler *handler,
				struct input_dev *dev,
@@ -99,10 +119,17 @@ static void kgdboc_restore_input_helper(struct work_struct *dummy)

static DECLARE_WORK(kgdboc_restore_input_work, kgdboc_restore_input_helper);

static void kgdboc_queue_restore_input_helper(struct irq_work *unused)
{
	schedule_work(&kgdboc_restore_input_work);
}

static DEFINE_IRQ_WORK(kgdboc_restore_input_irq_work, kgdboc_queue_restore_input_helper);

static void kgdboc_restore_input(void)
{
	if (likely(system_state == SYSTEM_RUNNING))
		schedule_work(&kgdboc_restore_input_work);
		irq_work_queue(&kgdboc_restore_input_irq_work);
}

static int kgdboc_register_kbd(char **cptr)
@@ -133,6 +160,7 @@ static void kgdboc_unregister_kbd(void)
			i--;
		}
	}
	irq_work_sync(&kgdboc_restore_input_irq_work);
	flush_work(&kgdboc_restore_input_work);
}
#else /* ! CONFIG_KDB_KEYBOARD */
+78 −75
Original line number Diff line number Diff line
@@ -184,6 +184,33 @@ char kdb_getchar(void)
	unreachable();
}

/**
 * kdb_position_cursor() - Place cursor in the correct horizontal position
 * @prompt: Nil-terminated string containing the prompt string
 * @buffer: Nil-terminated string containing the entire command line
 * @cp: Cursor position, pointer the character in buffer where the cursor
 *      should be positioned.
 *
 * The cursor is positioned by sending a carriage-return and then printing
 * the content of the line until we reach the correct cursor position.
 *
 * There is some additional fine detail here.
 *
 * Firstly, even though kdb_printf() will correctly format zero-width fields
 * we want the second call to kdb_printf() to be conditional. That keeps things
 * a little cleaner when LOGGING=1.
 *
 * Secondly, we can't combine everything into one call to kdb_printf() since
 * that renders into a fixed length buffer and the combined print could result
 * in unwanted truncation.
 */
static void kdb_position_cursor(char *prompt, char *buffer, char *cp)
{
	kdb_printf("\r%s", kdb_prompt_str);
	if (cp > buffer)
		kdb_printf("%.*s", (int)(cp - buffer), buffer);
}

/*
 * kdb_read
 *
@@ -220,8 +247,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
	int count;
	int i;
	int diag, dtab_count;
	int key, buf_size, ret;

	int key, ret;

	diag = kdbgetintenv("DTABCOUNT", &dtab_count);
	if (diag)
@@ -243,18 +269,11 @@ static char *kdb_read(char *buffer, size_t bufsize)
	switch (key) {
	case 8: /* backspace */
		if (cp > buffer) {
			if (cp < lastchar) {
				memcpy(tmpbuffer, cp, lastchar - cp);
				memcpy(cp-1, tmpbuffer, lastchar - cp);
			}
			*(--lastchar) = '\0';
			--cp;
			kdb_printf("\b%s \r", cp);
			tmp = *cp;
			*cp = '\0';
			kdb_printf(kdb_prompt_str);
			kdb_printf("%s", buffer);
			*cp = tmp;
			memmove(cp-1, cp, lastchar - cp + 1);
			lastchar--;
			cp--;
			kdb_printf("\b%s ", cp);
			kdb_position_cursor(kdb_prompt_str, buffer, cp);
		}
		break;
	case 10: /* linefeed */
@@ -269,22 +288,16 @@ static char *kdb_read(char *buffer, size_t bufsize)
		return buffer;
	case 4: /* Del */
		if (cp < lastchar) {
			memcpy(tmpbuffer, cp+1, lastchar - cp - 1);
			memcpy(cp, tmpbuffer, lastchar - cp - 1);
			*(--lastchar) = '\0';
			kdb_printf("%s \r", cp);
			tmp = *cp;
			*cp = '\0';
			kdb_printf(kdb_prompt_str);
			kdb_printf("%s", buffer);
			*cp = tmp;
			memmove(cp, cp+1, lastchar - cp);
			lastchar--;
			kdb_printf("%s ", cp);
			kdb_position_cursor(kdb_prompt_str, buffer, cp);
		}
		break;
	case 1: /* Home */
		if (cp > buffer) {
			kdb_printf("\r");
			kdb_printf(kdb_prompt_str);
			cp = buffer;
			kdb_position_cursor(kdb_prompt_str, buffer, cp);
		}
		break;
	case 5: /* End */
@@ -300,11 +313,10 @@ static char *kdb_read(char *buffer, size_t bufsize)
		}
		break;
	case 14: /* Down */
		memset(tmpbuffer, ' ',
		       strlen(kdb_prompt_str) + (lastchar-buffer));
		*(tmpbuffer+strlen(kdb_prompt_str) +
		  (lastchar-buffer)) = '\0';
		kdb_printf("\r%s\r", tmpbuffer);
	case 16: /* Up */
		kdb_printf("\r%*c\r",
			   (int)(strlen(kdb_prompt_str) + (lastchar - buffer)),
			   ' ');
		*lastchar = (char)key;
		*(lastchar+1) = '\0';
		return lastchar;
@@ -314,33 +326,19 @@ static char *kdb_read(char *buffer, size_t bufsize)
			++cp;
		}
		break;
	case 16: /* Up */
		memset(tmpbuffer, ' ',
		       strlen(kdb_prompt_str) + (lastchar-buffer));
		*(tmpbuffer+strlen(kdb_prompt_str) +
		  (lastchar-buffer)) = '\0';
		kdb_printf("\r%s\r", tmpbuffer);
		*lastchar = (char)key;
		*(lastchar+1) = '\0';
		return lastchar;
	case 9: /* Tab */
		if (tab < 2)
			++tab;
		p_tmp = buffer;
		while (*p_tmp == ' ')
			p_tmp++;
		if (p_tmp > cp)
			break;
		memcpy(tmpbuffer, p_tmp, cp-p_tmp);
		*(tmpbuffer + (cp-p_tmp)) = '\0';
		p_tmp = strrchr(tmpbuffer, ' ');
		if (p_tmp)
			++p_tmp;
		else
			p_tmp = tmpbuffer;
		len = strlen(p_tmp);
		buf_size = sizeof(tmpbuffer) - (p_tmp - tmpbuffer);
		count = kallsyms_symbol_complete(p_tmp, buf_size);

		tmp = *cp;
		*cp = '\0';
		p_tmp = strrchr(buffer, ' ');
		p_tmp = (p_tmp ? p_tmp + 1 : buffer);
		strscpy(tmpbuffer, p_tmp, sizeof(tmpbuffer));
		*cp = tmp;

		len = strlen(tmpbuffer);
		count = kallsyms_symbol_complete(tmpbuffer, sizeof(tmpbuffer));
		if (tab == 2 && count > 0) {
			kdb_printf("\n%d symbols are found.", count);
			if (count > dtab_count) {
@@ -352,46 +350,51 @@ static char *kdb_read(char *buffer, size_t bufsize)
			}
			kdb_printf("\n");
			for (i = 0; i < count; i++) {
				ret = kallsyms_symbol_next(p_tmp, i, buf_size);
				ret = kallsyms_symbol_next(tmpbuffer, i, sizeof(tmpbuffer));
				if (WARN_ON(!ret))
					break;
				if (ret != -E2BIG)
					kdb_printf("%s ", p_tmp);
					kdb_printf("%s ", tmpbuffer);
				else
					kdb_printf("%s... ", p_tmp);
				*(p_tmp + len) = '\0';
					kdb_printf("%s... ", tmpbuffer);
				tmpbuffer[len] = '\0';
			}
			if (i >= dtab_count)
				kdb_printf("...");
			kdb_printf("\n");
			kdb_printf(kdb_prompt_str);
			kdb_printf("%s", buffer);
			if (cp != lastchar)
				kdb_position_cursor(kdb_prompt_str, buffer, cp);
		} 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;
			/* How many new characters do we want from tmpbuffer? */
			len_tmp = strlen(tmpbuffer) - len;
			if (lastchar + len_tmp >= bufend)
				len_tmp = bufend - lastchar;

			if (len_tmp) {
				/* + 1 ensures the '\0' is memmove'd */
				memmove(cp+len_tmp, cp, (lastchar-cp) + 1);
				memcpy(cp, tmpbuffer+len, len_tmp);
				kdb_printf("%s", cp);
			cp += len;
			lastchar += len;
				cp += len_tmp;
				lastchar += len_tmp;
				if (cp != lastchar)
					kdb_position_cursor(kdb_prompt_str,
							    buffer, cp);
			}
		}
		kdb_nextline = 1; /* reset output line number */
		break;
	default:
		if (key >= 32 && lastchar < bufend) {
			if (cp < lastchar) {
				memcpy(tmpbuffer, cp, lastchar - cp);
				memcpy(cp+1, tmpbuffer, lastchar - cp);
				*++lastchar = '\0';
				memmove(cp+1, cp, lastchar - cp + 1);
				lastchar++;
				*cp = key;
				kdb_printf("%s\r", cp);
				kdb_printf("%s", cp);
				++cp;
				tmp = *cp;
				*cp = '\0';
				kdb_printf(kdb_prompt_str);
				kdb_printf("%s", buffer);
				*cp = tmp;
				kdb_position_cursor(kdb_prompt_str, buffer, cp);
			} else {
				*++lastchar = '\0';
				*cp++ = key;
+1 −1
Original line number Diff line number Diff line
@@ -2517,7 +2517,7 @@ static int kdb_summary(int argc, const char **argv)
	if (val.uptime > (24*60*60)) {
		int days = val.uptime / (24*60*60);
		val.uptime %= (24*60*60);
		kdb_printf("%d day%s ", days, days == 1 ? "" : "s");
		kdb_printf("%d day%s ", days, str_plural(days));
	}
	kdb_printf("%02ld:%02ld\n", val.uptime/(60*60), (val.uptime/60)%60);