mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [git pull] kgdb 2.6.27-rc7 fixes
@ 2008-09-26 18:05 Jason Wessel
  2008-09-26 18:05 ` [PATCH 1/5] kgdb: could not write to the last of valid memory with kgdb Jason Wessel
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wessel @ 2008-09-26 18:05 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

Linus, please pull the kgdb git tree fixes for 2.6.27-rc7

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_linus

Summary: 

Several regressions have been fixed for which there was no automated test.
 - Fix regression in flushing data/instruction cache on boundaries
 - User space single stepping should be ignored by kgdb
 - x86_64 had the BX and DX registers reversed in gdb
 - x86_64 eflags, CS, SS, registers were not properly passed to gdb
 - tty_io.c has one function repaired which is only used kgdboc
   to hook up to the tty system.  The input args were not properly
   checked.  Test case was:
     "echo ffff0 > /sys/module/kgdboc/parameters/kgdboc"
   

Short log follows:

--- 

Atsuo Igarashi (1):
      kgdb: could not write to the last of valid memory with kgdb

Jason Wessel (4):
      kgdb, x86, arm, mips, powerpc: ignore user space single stepping
      kgdb, x86_64: gdb serial has BX and DX reversed
      kgdb, x86_64: fix PS CS SS registers in gdb serial
      kgdboc,tty: Fix tty polling search to use name correctly

 arch/arm/kernel/kgdb.c     |    2 --
 arch/mips/kernel/kgdb.c    |    3 +--
 arch/powerpc/kernel/kgdb.c |    5 ++---
 arch/x86/kernel/kgdb.c     |   43 ++++++++++++++++++++++++++++++++-----------
 drivers/char/tty_io.c      |   14 ++++++++++++--
 include/asm-x86/kgdb.h     |   24 +++++++++++-------------
 kernel/kgdb.c              |   10 +++++-----
 7 files changed, 63 insertions(+), 38 deletions(-)

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

* [PATCH 1/5] kgdb: could not write to the last of valid memory with kgdb
  2008-09-26 18:05 [git pull] kgdb 2.6.27-rc7 fixes Jason Wessel
@ 2008-09-26 18:05 ` Jason Wessel
  2008-09-26 18:05   ` [PATCH 2/5] kgdb, x86, arm, mips, powerpc: ignore user space single stepping Jason Wessel
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wessel @ 2008-09-26 18:05 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

From: Atsuo Igarashi <atsuo_igarashi@tripeaks.co.jp>

On the ARM architecture, kgdb will crash the kernel if the last byte
of valid memory is written due to a flush_icache_range flushing
beyond the memory boundary.

Signed-off-by: Atsuo Igarashi <atsuo_igarashi@tripeaks.co.jp>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/kgdb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index eaa21fc..949806a 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -488,7 +488,7 @@ static int write_mem_msg(int binary)
 		if (err)
 			return err;
 		if (CACHE_FLUSH_IS_SAFE)
-			flush_icache_range(addr, addr + length + 1);
+			flush_icache_range(addr, addr + length);
 		return 0;
 	}
 
-- 
1.6.0.90.g436ed.dirty


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

* [PATCH 2/5] kgdb, x86, arm, mips, powerpc: ignore user space single stepping
  2008-09-26 18:05 ` [PATCH 1/5] kgdb: could not write to the last of valid memory with kgdb Jason Wessel
@ 2008-09-26 18:05   ` Jason Wessel
  2008-09-26 18:05     ` [PATCH 3/5] kgdb, x86_64: gdb serial has BX and DX reversed Jason Wessel
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wessel @ 2008-09-26 18:05 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

On the x86 arch, user space single step exceptions should be ignored
if they occur in the kernel space, such as ptrace stepping through a
system call.

First check if it is kgdb that is executing a single step, then ensure
it is not an accidental traversal into the user space, while in kgdb,
any other time the TIF_SINGLESTEP is set, kgdb should ignore the
exception.

On x86, arm, mips and powerpc, the kgdb_contthread usage was
inconsistent with the way single stepping is implemented in the kgdb
core.  The arch specific stub should always set the
kgdb_cpu_doing_single_step correctly if it is single stepping.  This
allows kgdb to correctly process an instruction steps if ptrace
happens to be requesting an instruction step over a system call.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 arch/arm/kernel/kgdb.c     |    2 --
 arch/mips/kernel/kgdb.c    |    3 +--
 arch/powerpc/kernel/kgdb.c |    5 ++---
 arch/x86/kernel/kgdb.c     |   18 +++++++++++-------
 kernel/kgdb.c              |    8 ++++----
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
index aaffaec..ba8ccfe 100644
--- a/arch/arm/kernel/kgdb.c
+++ b/arch/arm/kernel/kgdb.c
@@ -111,8 +111,6 @@ int kgdb_arch_handle_exception(int exception_vector, int signo,
 	case 'D':
 	case 'k':
 	case 'c':
-		kgdb_contthread = NULL;
-
 		/*
 		 * Try to read optional parameter, pc unchanged if no parm.
 		 * If this was a compiled breakpoint, we need to move
diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
index 8f6d58e..6e152c8 100644
--- a/arch/mips/kernel/kgdb.c
+++ b/arch/mips/kernel/kgdb.c
@@ -236,8 +236,7 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
 
 		atomic_set(&kgdb_cpu_doing_single_step, -1);
 		if (remcom_in_buffer[0] == 's')
-			if (kgdb_contthread)
-				atomic_set(&kgdb_cpu_doing_single_step, cpu);
+			atomic_set(&kgdb_cpu_doing_single_step, cpu);
 
 		return 0;
 	}
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index b4fdf2f..fe8f71d 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -347,9 +347,8 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
 			linux_regs->msr |= MSR_SE;
 #endif
 			kgdb_single_step = 1;
-			if (kgdb_contthread)
-				atomic_set(&kgdb_cpu_doing_single_step,
-					   raw_smp_processor_id());
+			atomic_set(&kgdb_cpu_doing_single_step,
+				   raw_smp_processor_id());
 		}
 		return 0;
 	}
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index f47f0eb..00f7896 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -378,10 +378,8 @@ int kgdb_arch_handle_exception(int e_vector, int signo, int err_code,
 		if (remcomInBuffer[0] == 's') {
 			linux_regs->flags |= X86_EFLAGS_TF;
 			kgdb_single_step = 1;
-			if (kgdb_contthread) {
-				atomic_set(&kgdb_cpu_doing_single_step,
-					   raw_smp_processor_id());
-			}
+			atomic_set(&kgdb_cpu_doing_single_step,
+				   raw_smp_processor_id());
 		}
 
 		get_debugreg(dr6, 6);
@@ -466,9 +464,15 @@ static int __kgdb_notify(struct die_args *args, unsigned long cmd)
 
 	case DIE_DEBUG:
 		if (atomic_read(&kgdb_cpu_doing_single_step) ==
-			raw_smp_processor_id() &&
-			user_mode(regs))
-			return single_step_cont(regs, args);
+		    raw_smp_processor_id()) {
+			if (user_mode(regs))
+				return single_step_cont(regs, args);
+			break;
+		} else if (test_thread_flag(TIF_SINGLESTEP))
+			/* This means a user thread is single stepping
+			 * a system call which should be ignored
+			 */
+			return NOTIFY_DONE;
 		/* fall through */
 	default:
 		if (user_mode(regs))
diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index 949806a..25d955d 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -1462,7 +1462,7 @@ acquirelock:
 	 * Get the passive CPU lock which will hold all the non-primary
 	 * CPU in a spin state while the debugger is active
 	 */
-	if (!kgdb_single_step || !kgdb_contthread) {
+	if (!kgdb_single_step) {
 		for (i = 0; i < NR_CPUS; i++)
 			atomic_set(&passive_cpu_wait[i], 1);
 	}
@@ -1475,7 +1475,7 @@ acquirelock:
 
 #ifdef CONFIG_SMP
 	/* Signal the other CPUs to enter kgdb_wait() */
-	if ((!kgdb_single_step || !kgdb_contthread) && kgdb_do_roundup)
+	if ((!kgdb_single_step) && kgdb_do_roundup)
 		kgdb_roundup_cpus(flags);
 #endif
 
@@ -1494,7 +1494,7 @@ acquirelock:
 	kgdb_post_primary_code(ks->linux_regs, ks->ex_vector, ks->err_code);
 	kgdb_deactivate_sw_breakpoints();
 	kgdb_single_step = 0;
-	kgdb_contthread = NULL;
+	kgdb_contthread = current;
 	exception_level = 0;
 
 	/* Talk to debugger with gdbserial protocol */
@@ -1508,7 +1508,7 @@ acquirelock:
 	kgdb_info[ks->cpu].task = NULL;
 	atomic_set(&cpu_in_kgdb[ks->cpu], 0);
 
-	if (!kgdb_single_step || !kgdb_contthread) {
+	if (!kgdb_single_step) {
 		for (i = NR_CPUS-1; i >= 0; i--)
 			atomic_set(&passive_cpu_wait[i], 0);
 		/*
-- 
1.6.0.90.g436ed.dirty


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

* [PATCH 3/5] kgdb, x86_64: gdb serial has BX and DX reversed
  2008-09-26 18:05   ` [PATCH 2/5] kgdb, x86, arm, mips, powerpc: ignore user space single stepping Jason Wessel
@ 2008-09-26 18:05     ` Jason Wessel
  2008-09-26 18:05       ` [PATCH 4/5] kgdb, x86_64: fix PS CS SS registers in gdb serial Jason Wessel
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wessel @ 2008-09-26 18:05 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

The BX and DX registers in the gdb serial register packet need to be
flipped for gdb to receive the correct data.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 include/asm-x86/kgdb.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-x86/kgdb.h b/include/asm-x86/kgdb.h
index 484c475..e86b306 100644
--- a/include/asm-x86/kgdb.h
+++ b/include/asm-x86/kgdb.h
@@ -42,9 +42,9 @@ enum regnames {
 #else /* ! CONFIG_X86_32 */
 enum regnames {
 	GDB_AX,			/* 0 */
-	GDB_DX,			/* 1 */
+	GDB_BX,			/* 1 */
 	GDB_CX,			/* 2 */
-	GDB_BX,			/* 3 */
+	GDB_DX,			/* 3 */
 	GDB_SI,			/* 4 */
 	GDB_DI,			/* 5 */
 	GDB_BP,			/* 6 */
-- 
1.6.0.90.g436ed.dirty


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

* [PATCH 4/5] kgdb, x86_64: fix PS CS SS registers in gdb serial
  2008-09-26 18:05     ` [PATCH 3/5] kgdb, x86_64: gdb serial has BX and DX reversed Jason Wessel
@ 2008-09-26 18:05       ` Jason Wessel
  2008-09-26 18:05         ` [PATCH 5/5] kgdboc,tty: Fix tty polling search to use name correctly Jason Wessel
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Wessel @ 2008-09-26 18:05 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

On x86_64 the gdb serial register structure defines the PS (also known
as eflags), CS and SS registers as 4 bytes entities.

This patch splits the x86_64 regnames enum into a 32 and 64 version to
account for the 32 bit entities in the gdb serial packets.

Also the program counter is properly filled in for the sleeping
threads.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 arch/x86/kernel/kgdb.c |   25 +++++++++++++++++++++----
 include/asm-x86/kgdb.h |   20 +++++++++-----------
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 00f7896..8282a21 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -69,6 +69,9 @@ static int gdb_x86vector = -1;
  */
 void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 {
+#ifndef CONFIG_X86_32
+	u32 *gdb_regs32 = (u32 *)gdb_regs;
+#endif
 	gdb_regs[GDB_AX]	= regs->ax;
 	gdb_regs[GDB_BX]	= regs->bx;
 	gdb_regs[GDB_CX]	= regs->cx;
@@ -76,9 +79,9 @@ void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 	gdb_regs[GDB_SI]	= regs->si;
 	gdb_regs[GDB_DI]	= regs->di;
 	gdb_regs[GDB_BP]	= regs->bp;
-	gdb_regs[GDB_PS]	= regs->flags;
 	gdb_regs[GDB_PC]	= regs->ip;
 #ifdef CONFIG_X86_32
+	gdb_regs[GDB_PS]	= regs->flags;
 	gdb_regs[GDB_DS]	= regs->ds;
 	gdb_regs[GDB_ES]	= regs->es;
 	gdb_regs[GDB_CS]	= regs->cs;
@@ -94,6 +97,9 @@ void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 	gdb_regs[GDB_R13]	= regs->r13;
 	gdb_regs[GDB_R14]	= regs->r14;
 	gdb_regs[GDB_R15]	= regs->r15;
+	gdb_regs32[GDB_PS]	= regs->flags;
+	gdb_regs32[GDB_CS]	= regs->cs;
+	gdb_regs32[GDB_SS]	= regs->ss;
 #endif
 	gdb_regs[GDB_SP]	= regs->sp;
 }
@@ -112,6 +118,9 @@ void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
  */
 void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p)
 {
+#ifndef CONFIG_X86_32
+	u32 *gdb_regs32 = (u32 *)gdb_regs;
+#endif
 	gdb_regs[GDB_AX]	= 0;
 	gdb_regs[GDB_BX]	= 0;
 	gdb_regs[GDB_CX]	= 0;
@@ -129,8 +138,10 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p)
 	gdb_regs[GDB_FS]	= 0xFFFF;
 	gdb_regs[GDB_GS]	= 0xFFFF;
 #else
-	gdb_regs[GDB_PS]	= *(unsigned long *)(p->thread.sp + 8);
-	gdb_regs[GDB_PC]	= 0;
+	gdb_regs32[GDB_PS]	= *(unsigned long *)(p->thread.sp + 8);
+	gdb_regs32[GDB_CS]	= __KERNEL_CS;
+	gdb_regs32[GDB_SS]	= __KERNEL_DS;
+	gdb_regs[GDB_PC]	= p->thread.ip;
 	gdb_regs[GDB_R8]	= 0;
 	gdb_regs[GDB_R9]	= 0;
 	gdb_regs[GDB_R10]	= 0;
@@ -153,6 +164,9 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p)
  */
 void gdb_regs_to_pt_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 {
+#ifndef CONFIG_X86_32
+	u32 *gdb_regs32 = (u32 *)gdb_regs;
+#endif
 	regs->ax		= gdb_regs[GDB_AX];
 	regs->bx		= gdb_regs[GDB_BX];
 	regs->cx		= gdb_regs[GDB_CX];
@@ -160,9 +174,9 @@ void gdb_regs_to_pt_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 	regs->si		= gdb_regs[GDB_SI];
 	regs->di		= gdb_regs[GDB_DI];
 	regs->bp		= gdb_regs[GDB_BP];
-	regs->flags		= gdb_regs[GDB_PS];
 	regs->ip		= gdb_regs[GDB_PC];
 #ifdef CONFIG_X86_32
+	regs->flags		= gdb_regs[GDB_PS];
 	regs->ds		= gdb_regs[GDB_DS];
 	regs->es		= gdb_regs[GDB_ES];
 	regs->cs		= gdb_regs[GDB_CS];
@@ -175,6 +189,9 @@ void gdb_regs_to_pt_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 	regs->r13		= gdb_regs[GDB_R13];
 	regs->r14		= gdb_regs[GDB_R14];
 	regs->r15		= gdb_regs[GDB_R15];
+	regs->flags		= gdb_regs32[GDB_PS];
+	regs->cs		= gdb_regs32[GDB_CS];
+	regs->ss		= gdb_regs32[GDB_SS];
 #endif
 }
 
diff --git a/include/asm-x86/kgdb.h b/include/asm-x86/kgdb.h
index e86b306..94d63db 100644
--- a/include/asm-x86/kgdb.h
+++ b/include/asm-x86/kgdb.h
@@ -39,8 +39,9 @@ enum regnames {
 	GDB_FS,			/* 14 */
 	GDB_GS,			/* 15 */
 };
+#define NUMREGBYTES		((GDB_GS+1)*4)
 #else /* ! CONFIG_X86_32 */
-enum regnames {
+enum regnames64 {
 	GDB_AX,			/* 0 */
 	GDB_BX,			/* 1 */
 	GDB_CX,			/* 2 */
@@ -58,18 +59,15 @@ enum regnames {
 	GDB_R14,		/* 14 */
 	GDB_R15,		/* 15 */
 	GDB_PC,			/* 16 */
-	GDB_PS,			/* 17 */
 };
-#endif /* CONFIG_X86_32 */
 
-/*
- * Number of bytes of registers:
- */
-#ifdef CONFIG_X86_32
-# define NUMREGBYTES		64
-#else
-# define NUMREGBYTES		((GDB_PS+1)*8)
-#endif
+enum regnames32 {
+	GDB_PS = 34,
+	GDB_CS,
+	GDB_SS,
+};
+#define NUMREGBYTES		((GDB_SS+1)*4)
+#endif /* CONFIG_X86_32 */
 
 static inline void arch_kgdb_breakpoint(void)
 {
-- 
1.6.0.90.g436ed.dirty


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

* [PATCH 5/5] kgdboc,tty: Fix tty polling search to use name correctly
  2008-09-26 18:05       ` [PATCH 4/5] kgdb, x86_64: fix PS CS SS registers in gdb serial Jason Wessel
@ 2008-09-26 18:05         ` Jason Wessel
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Wessel @ 2008-09-26 18:05 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

The tty_find_polling_driver() routine did not correctly check the base
part of the tty name.  This can lead to kgdboc selecting an incorrect
driver, as well as accepting a completely invalid tty such as "echo
ffff0 > /sys/module/kgdboc/parameters/kgdboc".

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/tty_io.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index daeb8f7..e4dce87 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -695,13 +695,23 @@ struct tty_driver *tty_find_polling_driver(char *name, int *line)
 {
 	struct tty_driver *p, *res = NULL;
 	int tty_line = 0;
+	int len;
 	char *str;
 
+	for (str = name; *str; str++)
+		if ((*str >= '0' && *str <= '9') || *str == ',')
+			break;
+	if (!*str)
+		return NULL;
+
+	len = str - name;
+	tty_line = simple_strtoul(str, &str, 10);
+
 	mutex_lock(&tty_mutex);
 	/* Search through the tty devices to look for a match */
 	list_for_each_entry(p, &tty_drivers, tty_drivers) {
-		str = name + strlen(p->name);
-		tty_line = simple_strtoul(str, &str, 10);
+		if (strncmp(name, p->name, len) != 0)
+			continue;
 		if (*str == ',')
 			str++;
 		if (*str == '\0')
-- 
1.6.0.90.g436ed.dirty


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

end of thread, other threads:[~2008-09-26 18:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-26 18:05 [git pull] kgdb 2.6.27-rc7 fixes Jason Wessel
2008-09-26 18:05 ` [PATCH 1/5] kgdb: could not write to the last of valid memory with kgdb Jason Wessel
2008-09-26 18:05   ` [PATCH 2/5] kgdb, x86, arm, mips, powerpc: ignore user space single stepping Jason Wessel
2008-09-26 18:05     ` [PATCH 3/5] kgdb, x86_64: gdb serial has BX and DX reversed Jason Wessel
2008-09-26 18:05       ` [PATCH 4/5] kgdb, x86_64: fix PS CS SS registers in gdb serial Jason Wessel
2008-09-26 18:05         ` [PATCH 5/5] kgdboc,tty: Fix tty polling search to use name correctly Jason Wessel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®