All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints
@ 2020-07-16 15:19 Daniel Thompson
  2020-07-16 15:19 ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist " Daniel Thompson
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Daniel Thompson @ 2020-07-16 15:19 UTC (permalink / raw)
  To: Jason Wessel, Douglas Anderson
  Cc: Daniel Thompson, Peter Zijlstra, sumit.garg, pmladek,
	sergey.senozhatsky, will, Masami Hiramatsu, kgdb-bugreport,
	linux-kernel, patches

kgdb has traditionally adopted a no safety rails approach to breakpoint
placement. If the debugger is commanded to place a breakpoint at an
address then it will do so even if that breakpoint results in kgdb
becoming inoperable.

A stop-the-world debugger with memory peek/poke intrinsically provides
its operator with the means to hose their system in all manner of
exciting ways (not least because stopping-the-world is already a DoS
attack ;-) ). Nevertheless the current no safety rail approach is
difficult to defend, especially given kprobes can provide us with plenty
of machinery to mark the parts of the kernel where breakpointing is
discouraged.

This patchset introduces some safety rails by using the existing kprobes
infrastructure and ensures this will be enabled by default on
architectures that implement kprobes. At present it does not cover
absolutely all locations where breakpoints can cause trouble but it will
block off several avenues, including the architecture specific parts
that are handled by arch_within_kprobe_blacklist().


Daniel Thompson (3):
  kgdb: Honour the kprobe blocklist when setting breakpoints
  kgdb: Use the kprobe blocklist to limit single stepping
  kgdb: Add NOKPROBE labels on the trap handler functions

 include/linux/kgdb.h        | 19 +++++++++++++++++++
 kernel/debug/debug_core.c   | 25 +++++++++++++++++++++++++
 kernel/debug/gdbstub.c      | 10 +++++++++-
 kernel/debug/kdb/kdb_bp.c   | 17 +++++++++++------
 kernel/debug/kdb/kdb_main.c | 10 ++++++++--
 lib/Kconfig.kgdb            | 14 ++++++++++++++
 6 files changed, 86 insertions(+), 9 deletions(-)

--
2.25.4


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

* [PATCH v2 1/3] kgdb: Honour the kprobe blocklist when setting breakpoints
  2020-07-16 15:19 [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Daniel Thompson
@ 2020-07-16 15:19 ` Daniel Thompson
  2020-07-16 20:48   ` kernel test robot
  2020-07-17 22:39   ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist when setting breakpoints Doug Anderson
  2020-07-16 15:19 ` [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping Daniel Thompson
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 17+ messages in thread
From: Daniel Thompson @ 2020-07-16 15:19 UTC (permalink / raw)
  To: Jason Wessel, Douglas Anderson
  Cc: Daniel Thompson, Peter Zijlstra, sumit.garg, pmladek,
	sergey.senozhatsky, will, Masami Hiramatsu, kgdb-bugreport,
	linux-kernel, patches

Currently kgdb has absolutely no safety rails in place to discourage or
prevent a user from placing a breakpoint in dangerous places such as
the debugger's own trap entry/exit and other places where it is not safe
to take synchronous traps.

Introduce a new config symbol KGDB_HONOUR_BLOCKLIST and modify the
default implementation of kgdb_validate_break_address() so that we use
the kprobe blocklist to prohibit instrumentation of critical functions
if the config symbol is set. The config symbol dependencies are set to
ensure that the blocklist will be enabled by default if we enable KGDB
and are compiling for an architecture where we HAVE_KPROBES.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 include/linux/kgdb.h      | 18 ++++++++++++++++++
 kernel/debug/debug_core.c |  4 ++++
 kernel/debug/kdb/kdb_bp.c |  9 +++++++++
 lib/Kconfig.kgdb          | 14 ++++++++++++++
 4 files changed, 45 insertions(+)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 529116b0cabe..7caba4604edc 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -16,6 +16,7 @@
 #include <linux/linkage.h>
 #include <linux/init.h>
 #include <linux/atomic.h>
+#include <linux/kprobes.h>
 #ifdef CONFIG_HAVE_ARCH_KGDB
 #include <asm/kgdb.h>
 #endif
@@ -323,6 +324,23 @@ extern int kgdb_nmicallin(int cpu, int trapnr, void *regs, int err_code,
 			  atomic_t *snd_rdy);
 extern void gdbstub_exit(int status);
 
+/*
+ * kgdb and kprobes both use the same (kprobe) blocklist (which makes sense
+ * given they are both typically hooked up to the same trap meaning on most
+ * architectures one cannot be used to debug the other)
+ *
+ * However on architectures where kprobes is not (yet) implemented we permit
+ * breakpoints everywhere rather than blocking everything by default.
+ */
+static inline bool kgdb_within_blocklist(unsigned long addr)
+{
+#ifdef CONFIG_KGDB_HONOUR_BLOCKLIST
+	return within_kprobe_blacklist(addr);
+#else
+	return false;
+#endif
+}
+
 extern int			kgdb_single_step;
 extern atomic_t			kgdb_active;
 #define in_dbg_master() \
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 9e5934780f41..133a361578dc 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -188,6 +188,10 @@ int __weak kgdb_validate_break_address(unsigned long addr)
 {
 	struct kgdb_bkpt tmp;
 	int err;
+
+	if (kgdb_within_blocklist(addr))
+		return -EINVAL;
+
 	/* Validate setting the breakpoint and then removing it.  If the
 	 * remove fails, the kernel needs to emit a bad message because we
 	 * are deep trouble not being able to put things back the way we
diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
index d7ebb2c79cb8..ec4940146612 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -306,6 +306,15 @@ static int kdb_bp(int argc, const char **argv)
 	if (!template.bp_addr)
 		return KDB_BADINT;
 
+	/*
+	 * This check is redundant (since the breakpoint machinery should
+	 * be doing the same check during kdb_bp_install) but gives the
+	 * user immediate feedback.
+	 */
+	diag = kgdb_validate_break_address(template.bp_addr);
+	if (diag)
+		return diag;
+
 	/*
 	 * Find an empty bp structure to allocate
 	 */
diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
index ffa7a76de086..9d0d408f81b1 100644
--- a/lib/Kconfig.kgdb
+++ b/lib/Kconfig.kgdb
@@ -19,6 +19,20 @@ menuconfig KGDB
 
 if KGDB
 
+config KGDB_HONOUR_BLOCKLIST
+	bool "KGDB: use kprobe blocklist to prohibit unsafe breakpoints"
+	depends on HAVE_KPROBES
+	select KPROBES
+	default y
+	help
+	  If set to Y the debug core will use the kprobe blocklist to
+	  identify symbols where it is unsafe to set breakpoints.
+	  In particular this disallows instrumentation of functions
+	  called during debug trap handling and thus makes it very
+	  difficult to inadvertently provoke recursive trap handling.
+
+	  If unsure, say Y.
+
 config KGDB_SERIAL_CONSOLE
 	tristate "KGDB: use kgdb over the serial console"
 	select CONSOLE_POLL
-- 
2.25.4


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

* [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping
  2020-07-16 15:19 [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Daniel Thompson
  2020-07-16 15:19 ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist " Daniel Thompson
@ 2020-07-16 15:19 ` Daniel Thompson
  2020-07-17 22:39   ` Doug Anderson
  2020-07-16 15:19 ` [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions Daniel Thompson
  2020-07-17 13:48 ` [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Masami Hiramatsu
  3 siblings, 1 reply; 17+ messages in thread
From: Daniel Thompson @ 2020-07-16 15:19 UTC (permalink / raw)
  To: Jason Wessel, Douglas Anderson
  Cc: Daniel Thompson, Peter Zijlstra, sumit.garg, pmladek,
	sergey.senozhatsky, will, Masami Hiramatsu, kgdb-bugreport,
	linux-kernel, patches

If we are running in a part of the kernel that dislikes breakpoint
debugging then it is very unlikely to be safe to single step. Add
some safety rails to prevent stepping through anything on the kprobe
blocklist.

As part of this kdb_ss() will no longer set the DOING_SS flags when it
requests a step. This is safe because this flag is already redundant,
returning KDB_CMD_SS is all that is needed to request a step (and this
saves us from having to unset the flag if the safety check fails).

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 include/linux/kgdb.h        |  1 +
 kernel/debug/debug_core.c   | 13 +++++++++++++
 kernel/debug/gdbstub.c      | 10 +++++++++-
 kernel/debug/kdb/kdb_bp.c   |  8 ++------
 kernel/debug/kdb/kdb_main.c | 10 ++++++++--
 5 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 7caba4604edc..aefe823998cb 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -214,6 +214,7 @@ extern void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc);
 
 /* Optional functions. */
 extern int kgdb_validate_break_address(unsigned long addr);
+extern int kgdb_validate_single_step_address(unsigned long addr);
 extern int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt);
 extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
 
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 133a361578dc..4b59bcc90c5d 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -208,6 +208,19 @@ int __weak kgdb_validate_break_address(unsigned long addr)
 	return err;
 }
 
+int __weak kgdb_validate_single_step_address(unsigned long addr)
+{
+	/*
+	 * Disallow stepping when we are executing code that is marked
+	 * as unsuitable for breakpointing... stepping won't be safe
+	 * either!
+	 */
+	if (kgdb_within_blocklist(addr))
+		return -EINVAL;
+
+	return 0;
+}
+
 unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs)
 {
 	return instruction_pointer(regs);
diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
index 61774aec46b4..f1c88007cc2b 100644
--- a/kernel/debug/gdbstub.c
+++ b/kernel/debug/gdbstub.c
@@ -1041,8 +1041,16 @@ int gdb_serial_stub(struct kgdb_state *ks)
 			if (tmp == 0)
 				break;
 			/* Fall through - on tmp < 0 */
-		case 'c': /* Continue packet */
 		case 's': /* Single step packet */
+			error = kgdb_validate_single_step_address(
+					kgdb_arch_pc(ks->ex_vector,
+						     ks->linux_regs));
+			if (error != 0) {
+				error_packet(remcom_out_buffer, error);
+				break;
+			}
+			fallthrough;
+		case 'c': /* Continue packet */
 			if (kgdb_contthread && kgdb_contthread != current) {
 				/* Can't switch threads in kgdb */
 				error_packet(remcom_out_buffer, -EINVAL);
diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
index ec4940146612..4853c413f579 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -507,18 +507,14 @@ static int kdb_bc(int argc, const char **argv)
  *	None.
  * Remarks:
  *
- *	Set the arch specific option to trigger a debug trap after the next
- *	instruction.
+ *	KDB_CMD_SS is a command that our caller acts on to effect the step.
  */
 
 static int kdb_ss(int argc, const char **argv)
 {
 	if (argc != 0)
 		return KDB_ARGCOUNT;
-	/*
-	 * Set trace flag and go.
-	 */
-	KDB_STATE_SET(DOING_SS);
+
 	return KDB_CMD_SS;
 }
 
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 5c7949061671..cd40bf780b93 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1189,7 +1189,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
 		     kdb_dbtrap_t db_result)
 {
 	char *cmdbuf;
-	int diag;
+	int diag, res;
 	struct task_struct *kdb_current =
 		kdb_curr_task(raw_smp_processor_id());
 
@@ -1346,10 +1346,16 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
 		}
 		if (diag == KDB_CMD_GO
 		 || diag == KDB_CMD_CPU
-		 || diag == KDB_CMD_SS
 		 || diag == KDB_CMD_KGDB)
 			break;
 
+		if (diag == KDB_CMD_SS) {
+			res = kgdb_validate_single_step_address(instruction_pointer(regs));
+			if (res == 0)
+				break;
+			diag = res;
+		}
+
 		if (diag)
 			kdb_cmderror(diag);
 	}
-- 
2.25.4


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

* [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions
  2020-07-16 15:19 [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Daniel Thompson
  2020-07-16 15:19 ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist " Daniel Thompson
  2020-07-16 15:19 ` [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping Daniel Thompson
@ 2020-07-16 15:19 ` Daniel Thompson
  2020-07-17 22:39   ` Doug Anderson
  2020-07-17 13:48 ` [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Masami Hiramatsu
  3 siblings, 1 reply; 17+ messages in thread
From: Daniel Thompson @ 2020-07-16 15:19 UTC (permalink / raw)
  To: Jason Wessel, Douglas Anderson
  Cc: Daniel Thompson, Peter Zijlstra, sumit.garg, pmladek,
	sergey.senozhatsky, will, Masami Hiramatsu, kgdb-bugreport,
	linux-kernel, patches

Currently kgdb honours the kprobe blocklist but doesn't place its own
trap handling code on the list. Add labels to discourage attempting to
use kgdb to debug itself.

These changes do not make it impossible to provoke recursive trapping
since they do not cover all the calls that can be made on kgdb's entry
logic. However going much further whilst we are sharing the kprobe
blocklist risks reducing the capabilities of kprobe and this would be a
bad trade off (especially so given kgdb's users are currently conditioned
to avoid recursive traps).

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 kernel/debug/debug_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 4b59bcc90c5d..b056afb1beec 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -183,6 +183,7 @@ int __weak kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
 	return copy_to_kernel_nofault((char *)bpt->bpt_addr,
 				  (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
 }
+NOKPROBE_SYMBOL(kgdb_arch_remove_breakpoint);
 
 int __weak kgdb_validate_break_address(unsigned long addr)
 {
@@ -315,6 +316,7 @@ static void kgdb_flush_swbreak_addr(unsigned long addr)
 	/* Force flush instruction cache if it was outside the mm */
 	flush_icache_range(addr, addr + BREAK_INSTR_SIZE);
 }
+NOKPROBE_SYMBOL(kgdb_flush_swbreak_addr);
 
 /*
  * SW breakpoint management:
@@ -405,6 +407,7 @@ int dbg_deactivate_sw_breakpoints(void)
 	}
 	return ret;
 }
+NOKPROBE_SYMBOL(dbg_deactivate_sw_breakpoints);
 
 int dbg_remove_sw_break(unsigned long addr)
 {
@@ -573,6 +576,7 @@ static int kgdb_reenter_check(struct kgdb_state *ks)
 
 	return 1;
 }
+NOKPROBE_SYMBOL(kgdb_reenter_check);
 
 static void dbg_touch_watchdogs(void)
 {
@@ -811,6 +815,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs,
 
 	return kgdb_info[cpu].ret_state;
 }
+NOKPROBE_SYMBOL(kgdb_cpu_enter);
 
 /*
  * kgdb_handle_exception() - main entry point from a kernel exception
@@ -855,6 +860,7 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 		arch_kgdb_ops.enable_nmi(1);
 	return ret;
 }
+NOKPROBE_SYMBOL(kgdb_handle_exception);
 
 /*
  * GDB places a breakpoint at this function to know dynamically loaded objects.
@@ -889,6 +895,7 @@ int kgdb_nmicallback(int cpu, void *regs)
 #endif
 	return 1;
 }
+NOKPROBE_SYMBOL(kgdb_nmicallback);
 
 int kgdb_nmicallin(int cpu, int trapnr, void *regs, int err_code,
 							atomic_t *send_ready)
@@ -914,6 +921,7 @@ int kgdb_nmicallin(int cpu, int trapnr, void *regs, int err_code,
 #endif
 	return 1;
 }
+NOKPROBE_SYMBOL(kgdb_nmicallin);
 
 static void kgdb_console_write(struct console *co, const char *s,
    unsigned count)
-- 
2.25.4


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

* Re: [PATCH v2 1/3] kgdb: Honour the kprobe blocklist when setting breakpoints
  2020-07-16 15:19 ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist " Daniel Thompson
@ 2020-07-16 20:48   ` kernel test robot
  2020-07-17  8:13     ` Masami Hiramatsu
  2020-07-17 22:39   ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist when setting breakpoints Doug Anderson
  1 sibling, 1 reply; 17+ messages in thread
From: kernel test robot @ 2020-07-16 20:48 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 38187 bytes --]

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on kgdb/kgdb-next pmladek/for-next v5.8-rc5 next-20200716]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Thompson/kgdb-Honour-the-kprobe-blacklist-when-setting-breakpoints/20200716-232506
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f8456690ba8eb18ea4714e68554e242a04f65cff
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/net/wireless/intel/ipw2x00/ipw2100.c:3794:16: error: conflicting types for 'show_registers'
    3794 | static ssize_t show_registers(struct device *d, struct device_attribute *attr,
         |                ^~~~~~~~~~~~~~
   In file included from include/linux/kgdb.h:19,
                    from arch/arm64/include/asm/cacheflush.h:11,
                    from include/linux/highmem.h:12,
                    from include/linux/pagemap.h:11,
                    from include/linux/blkdev.h:16,
                    from include/linux/blk-cgroup.h:23,
                    from include/linux/writeback.h:14,
                    from include/linux/memcontrol.h:22,
                    from include/net/sock.h:53,
                    from include/linux/tcp.h:19,
                    from drivers/net/wireless/intel/ipw2x00/ipw2100.c:144:
   include/linux/kprobes.h:230:13: note: previous declaration of 'show_registers' was here
     230 | extern void show_registers(struct pt_regs *regs);
         |             ^~~~~~~~~~~~~~

vim +/show_registers +3794 drivers/net/wireless/intel/ipw2x00/ipw2100.c

2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3558  
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3559  #define IPW2100_REG(x) { IPW_ ##x, #x }
c4aee8c21ff5d8d drivers/net/wireless/ipw2100.c               Jiri Benc        2005-08-25  3560  static const struct {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3561  	u32 addr;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3562  	const char *name;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3563  } hw_data[] = {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3564  IPW2100_REG(REG_GP_CNTRL),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3565  	    IPW2100_REG(REG_GPIO),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3566  	    IPW2100_REG(REG_INTA),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3567  	    IPW2100_REG(REG_INTA_MASK), IPW2100_REG(REG_RESET_REG),};
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3568  #define IPW2100_NIC(x, s) { x, #x, s }
c4aee8c21ff5d8d drivers/net/wireless/ipw2100.c               Jiri Benc        2005-08-25  3569  static const struct {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3570  	u32 addr;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3571  	const char *name;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3572  	size_t size;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3573  } nic_data[] = {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3574  IPW2100_NIC(IPW2100_CONTROL_REG, 2),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3575  	    IPW2100_NIC(0x210014, 1), IPW2100_NIC(0x210000, 1),};
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3576  #define IPW2100_ORD(x, d) { IPW_ORD_ ##x, #x, d }
c4aee8c21ff5d8d drivers/net/wireless/ipw2100.c               Jiri Benc        2005-08-25  3577  static const struct {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3578  	u8 index;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3579  	const char *name;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3580  	const char *desc;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3581  } ord_data[] = {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3582  IPW2100_ORD(STAT_TX_HOST_REQUESTS, "requested Host Tx's (MSDU)"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3583  	    IPW2100_ORD(STAT_TX_HOST_COMPLETE,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3584  				"successful Host Tx's (MSDU)"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3585  	    IPW2100_ORD(STAT_TX_DIR_DATA,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3586  				"successful Directed Tx's (MSDU)"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3587  	    IPW2100_ORD(STAT_TX_DIR_DATA1,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3588  				"successful Directed Tx's (MSDU) @ 1MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3589  	    IPW2100_ORD(STAT_TX_DIR_DATA2,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3590  				"successful Directed Tx's (MSDU) @ 2MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3591  	    IPW2100_ORD(STAT_TX_DIR_DATA5_5,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3592  				"successful Directed Tx's (MSDU) @ 5_5MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3593  	    IPW2100_ORD(STAT_TX_DIR_DATA11,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3594  				"successful Directed Tx's (MSDU) @ 11MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3595  	    IPW2100_ORD(STAT_TX_NODIR_DATA1,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3596  				"successful Non_Directed Tx's (MSDU) @ 1MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3597  	    IPW2100_ORD(STAT_TX_NODIR_DATA2,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3598  				"successful Non_Directed Tx's (MSDU) @ 2MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3599  	    IPW2100_ORD(STAT_TX_NODIR_DATA5_5,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3600  				"successful Non_Directed Tx's (MSDU) @ 5.5MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3601  	    IPW2100_ORD(STAT_TX_NODIR_DATA11,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3602  				"successful Non_Directed Tx's (MSDU) @ 11MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3603  	    IPW2100_ORD(STAT_NULL_DATA, "successful NULL data Tx's"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3604  	    IPW2100_ORD(STAT_TX_RTS, "successful Tx RTS"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3605  	    IPW2100_ORD(STAT_TX_CTS, "successful Tx CTS"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3606  	    IPW2100_ORD(STAT_TX_ACK, "successful Tx ACK"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3607  	    IPW2100_ORD(STAT_TX_ASSN, "successful Association Tx's"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3608  	    IPW2100_ORD(STAT_TX_ASSN_RESP,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3609  				"successful Association response Tx's"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3610  	    IPW2100_ORD(STAT_TX_REASSN,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3611  				"successful Reassociation Tx's"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3612  	    IPW2100_ORD(STAT_TX_REASSN_RESP,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3613  				"successful Reassociation response Tx's"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3614  	    IPW2100_ORD(STAT_TX_PROBE,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3615  				"probes successfully transmitted"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3616  	    IPW2100_ORD(STAT_TX_PROBE_RESP,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3617  				"probe responses successfully transmitted"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3618  	    IPW2100_ORD(STAT_TX_BEACON, "tx beacon"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3619  	    IPW2100_ORD(STAT_TX_ATIM, "Tx ATIM"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3620  	    IPW2100_ORD(STAT_TX_DISASSN,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3621  				"successful Disassociation TX"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3622  	    IPW2100_ORD(STAT_TX_AUTH, "successful Authentication Tx"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3623  	    IPW2100_ORD(STAT_TX_DEAUTH,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3624  				"successful Deauthentication TX"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3625  	    IPW2100_ORD(STAT_TX_TOTAL_BYTES,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3626  				"Total successful Tx data bytes"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3627  	    IPW2100_ORD(STAT_TX_RETRIES, "Tx retries"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3628  	    IPW2100_ORD(STAT_TX_RETRY1, "Tx retries at 1MBPS"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3629  	    IPW2100_ORD(STAT_TX_RETRY2, "Tx retries at 2MBPS"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3630  	    IPW2100_ORD(STAT_TX_RETRY5_5, "Tx retries at 5.5MBPS"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3631  	    IPW2100_ORD(STAT_TX_RETRY11, "Tx retries at 11MBPS"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3632  	    IPW2100_ORD(STAT_TX_FAILURES, "Tx Failures"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3633  	    IPW2100_ORD(STAT_TX_MAX_TRIES_IN_HOP,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3634  				"times max tries in a hop failed"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3635  	    IPW2100_ORD(STAT_TX_DISASSN_FAIL,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3636  				"times disassociation failed"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3637  	    IPW2100_ORD(STAT_TX_ERR_CTS, "missed/bad CTS frames"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3638  	    IPW2100_ORD(STAT_TX_ERR_ACK, "tx err due to acks"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3639  	    IPW2100_ORD(STAT_RX_HOST, "packets passed to host"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3640  	    IPW2100_ORD(STAT_RX_DIR_DATA, "directed packets"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3641  	    IPW2100_ORD(STAT_RX_DIR_DATA1, "directed packets at 1MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3642  	    IPW2100_ORD(STAT_RX_DIR_DATA2, "directed packets at 2MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3643  	    IPW2100_ORD(STAT_RX_DIR_DATA5_5,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3644  				"directed packets at 5.5MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3645  	    IPW2100_ORD(STAT_RX_DIR_DATA11, "directed packets at 11MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3646  	    IPW2100_ORD(STAT_RX_NODIR_DATA, "nondirected packets"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3647  	    IPW2100_ORD(STAT_RX_NODIR_DATA1,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3648  				"nondirected packets at 1MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3649  	    IPW2100_ORD(STAT_RX_NODIR_DATA2,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3650  				"nondirected packets at 2MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3651  	    IPW2100_ORD(STAT_RX_NODIR_DATA5_5,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3652  				"nondirected packets at 5.5MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3653  	    IPW2100_ORD(STAT_RX_NODIR_DATA11,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3654  				"nondirected packets at 11MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3655  	    IPW2100_ORD(STAT_RX_NULL_DATA, "null data rx's"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3656  	    IPW2100_ORD(STAT_RX_RTS, "Rx RTS"), IPW2100_ORD(STAT_RX_CTS,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3657  								    "Rx CTS"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3658  	    IPW2100_ORD(STAT_RX_ACK, "Rx ACK"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3659  	    IPW2100_ORD(STAT_RX_CFEND, "Rx CF End"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3660  	    IPW2100_ORD(STAT_RX_CFEND_ACK, "Rx CF End + CF Ack"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3661  	    IPW2100_ORD(STAT_RX_ASSN, "Association Rx's"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3662  	    IPW2100_ORD(STAT_RX_ASSN_RESP, "Association response Rx's"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3663  	    IPW2100_ORD(STAT_RX_REASSN, "Reassociation Rx's"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3664  	    IPW2100_ORD(STAT_RX_REASSN_RESP,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3665  				"Reassociation response Rx's"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3666  	    IPW2100_ORD(STAT_RX_PROBE, "probe Rx's"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3667  	    IPW2100_ORD(STAT_RX_PROBE_RESP, "probe response Rx's"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3668  	    IPW2100_ORD(STAT_RX_BEACON, "Rx beacon"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3669  	    IPW2100_ORD(STAT_RX_ATIM, "Rx ATIM"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3670  	    IPW2100_ORD(STAT_RX_DISASSN, "disassociation Rx"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3671  	    IPW2100_ORD(STAT_RX_AUTH, "authentication Rx"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3672  	    IPW2100_ORD(STAT_RX_DEAUTH, "deauthentication Rx"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3673  	    IPW2100_ORD(STAT_RX_TOTAL_BYTES,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3674  				"Total rx data bytes received"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3675  	    IPW2100_ORD(STAT_RX_ERR_CRC, "packets with Rx CRC error"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3676  	    IPW2100_ORD(STAT_RX_ERR_CRC1, "Rx CRC errors at 1MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3677  	    IPW2100_ORD(STAT_RX_ERR_CRC2, "Rx CRC errors at 2MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3678  	    IPW2100_ORD(STAT_RX_ERR_CRC5_5, "Rx CRC errors at 5.5MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3679  	    IPW2100_ORD(STAT_RX_ERR_CRC11, "Rx CRC errors at 11MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3680  	    IPW2100_ORD(STAT_RX_DUPLICATE1,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3681  				"duplicate rx packets at 1MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3682  	    IPW2100_ORD(STAT_RX_DUPLICATE2,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3683  				"duplicate rx packets at 2MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3684  	    IPW2100_ORD(STAT_RX_DUPLICATE5_5,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3685  				"duplicate rx packets at 5.5MB"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3686  	    IPW2100_ORD(STAT_RX_DUPLICATE11,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3687  				"duplicate rx packets at 11MB"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3688  	    IPW2100_ORD(STAT_RX_DUPLICATE, "duplicate rx packets"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3689  	    IPW2100_ORD(PERS_DB_LOCK, "locking fw permanent  db"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3690  	    IPW2100_ORD(PERS_DB_SIZE, "size of fw permanent  db"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3691  	    IPW2100_ORD(PERS_DB_ADDR, "address of fw permanent  db"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3692  	    IPW2100_ORD(STAT_RX_INVALID_PROTOCOL,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3693  				"rx frames with invalid protocol"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3694  	    IPW2100_ORD(SYS_BOOT_TIME, "Boot time"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3695  	    IPW2100_ORD(STAT_RX_NO_BUFFER,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3696  				"rx frames rejected due to no buffer"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3697  	    IPW2100_ORD(STAT_RX_MISSING_FRAG,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3698  				"rx frames dropped due to missing fragment"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3699  	    IPW2100_ORD(STAT_RX_ORPHAN_FRAG,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3700  				"rx frames dropped due to non-sequential fragment"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3701  	    IPW2100_ORD(STAT_RX_ORPHAN_FRAME,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3702  				"rx frames dropped due to unmatched 1st frame"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3703  	    IPW2100_ORD(STAT_RX_FRAG_AGEOUT,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3704  				"rx frames dropped due to uncompleted frame"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3705  	    IPW2100_ORD(STAT_RX_ICV_ERRORS,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3706  				"ICV errors during decryption"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3707  	    IPW2100_ORD(STAT_PSP_SUSPENSION, "times adapter suspended"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3708  	    IPW2100_ORD(STAT_PSP_BCN_TIMEOUT, "beacon timeout"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3709  	    IPW2100_ORD(STAT_PSP_POLL_TIMEOUT,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3710  				"poll response timeouts"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3711  	    IPW2100_ORD(STAT_PSP_NONDIR_TIMEOUT,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3712  				"timeouts waiting for last {broad,multi}cast pkt"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3713  	    IPW2100_ORD(STAT_PSP_RX_DTIMS, "PSP DTIMs received"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3714  	    IPW2100_ORD(STAT_PSP_RX_TIMS, "PSP TIMs received"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3715  	    IPW2100_ORD(STAT_PSP_STATION_ID, "PSP Station ID"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3716  	    IPW2100_ORD(LAST_ASSN_TIME, "RTC time of last association"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3717  	    IPW2100_ORD(STAT_PERCENT_MISSED_BCNS,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3718  				"current calculation of % missed beacons"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3719  	    IPW2100_ORD(STAT_PERCENT_RETRIES,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3720  				"current calculation of % missed tx retries"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3721  	    IPW2100_ORD(ASSOCIATED_AP_PTR,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3722  				"0 if not associated, else pointer to AP table entry"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3723  	    IPW2100_ORD(AVAILABLE_AP_CNT,
3ea0a58cf9cf66e drivers/net/wireless/intel/ipw2x00/ipw2100.c Colin Ian King   2018-04-28  3724  				"AP's described in the AP table"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3725  	    IPW2100_ORD(AP_LIST_PTR, "Ptr to list of available APs"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3726  	    IPW2100_ORD(STAT_AP_ASSNS, "associations"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3727  	    IPW2100_ORD(STAT_ASSN_FAIL, "association failures"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3728  	    IPW2100_ORD(STAT_ASSN_RESP_FAIL,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3729  				"failures due to response fail"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3730  	    IPW2100_ORD(STAT_FULL_SCANS, "full scans"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3731  	    IPW2100_ORD(CARD_DISABLED, "Card Disabled"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3732  	    IPW2100_ORD(STAT_ROAM_INHIBIT,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3733  				"times roaming was inhibited due to activity"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3734  	    IPW2100_ORD(RSSI_AT_ASSN,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3735  				"RSSI of associated AP at time of association"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3736  	    IPW2100_ORD(STAT_ASSN_CAUSE1,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3737  				"reassociation: no probe response or TX on hop"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3738  	    IPW2100_ORD(STAT_ASSN_CAUSE2,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3739  				"reassociation: poor tx/rx quality"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3740  	    IPW2100_ORD(STAT_ASSN_CAUSE3,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3741  				"reassociation: tx/rx quality (excessive AP load"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3742  	    IPW2100_ORD(STAT_ASSN_CAUSE4,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3743  				"reassociation: AP RSSI level"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3744  	    IPW2100_ORD(STAT_ASSN_CAUSE5,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3745  				"reassociations due to load leveling"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3746  	    IPW2100_ORD(STAT_AUTH_FAIL, "times authentication failed"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3747  	    IPW2100_ORD(STAT_AUTH_RESP_FAIL,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3748  				"times authentication response failed"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3749  	    IPW2100_ORD(STATION_TABLE_CNT,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3750  				"entries in association table"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3751  	    IPW2100_ORD(RSSI_AVG_CURR, "Current avg RSSI"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3752  	    IPW2100_ORD(POWER_MGMT_MODE, "Power mode - 0=CAM, 1=PSP"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3753  	    IPW2100_ORD(COUNTRY_CODE,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3754  				"IEEE country code as recv'd from beacon"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3755  	    IPW2100_ORD(COUNTRY_CHANNELS,
fd9071ec61db420 drivers/net/wireless/ipw2x00/ipw2100.c       Masanari Iida    2012-04-13  3756  				"channels supported by country"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3757  	    IPW2100_ORD(RESET_CNT, "adapter resets (warm)"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3758  	    IPW2100_ORD(BEACON_INTERVAL, "Beacon interval"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3759  	    IPW2100_ORD(ANTENNA_DIVERSITY,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3760  				"TRUE if antenna diversity is disabled"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3761  	    IPW2100_ORD(DTIM_PERIOD, "beacon intervals between DTIMs"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3762  	    IPW2100_ORD(OUR_FREQ,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3763  				"current radio freq lower digits - channel ID"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3764  	    IPW2100_ORD(RTC_TIME, "current RTC time"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3765  	    IPW2100_ORD(PORT_TYPE, "operating mode"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3766  	    IPW2100_ORD(CURRENT_TX_RATE, "current tx rate"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3767  	    IPW2100_ORD(SUPPORTED_RATES, "supported tx rates"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3768  	    IPW2100_ORD(ATIM_WINDOW, "current ATIM Window"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3769  	    IPW2100_ORD(BASIC_RATES, "basic tx rates"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3770  	    IPW2100_ORD(NIC_HIGHEST_RATE, "NIC highest tx rate"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3771  	    IPW2100_ORD(AP_HIGHEST_RATE, "AP highest tx rate"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3772  	    IPW2100_ORD(CAPABILITIES,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3773  				"Management frame capability field"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3774  	    IPW2100_ORD(AUTH_TYPE, "Type of authentication"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3775  	    IPW2100_ORD(RADIO_TYPE, "Adapter card platform type"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3776  	    IPW2100_ORD(RTS_THRESHOLD,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3777  				"Min packet length for RTS handshaking"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3778  	    IPW2100_ORD(INT_MODE, "International mode"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3779  	    IPW2100_ORD(FRAGMENTATION_THRESHOLD,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3780  				"protocol frag threshold"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3781  	    IPW2100_ORD(EEPROM_SRAM_DB_BLOCK_START_ADDRESS,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3782  				"EEPROM offset in SRAM"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3783  	    IPW2100_ORD(EEPROM_SRAM_DB_BLOCK_SIZE,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3784  				"EEPROM size in SRAM"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3785  	    IPW2100_ORD(EEPROM_SKU_CAPABILITY, "EEPROM SKU Capability"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3786  	    IPW2100_ORD(EEPROM_IBSS_11B_CHANNELS,
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3787  				"EEPROM IBSS 11b channel set"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3788  	    IPW2100_ORD(MAC_VERSION, "MAC Version"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3789  	    IPW2100_ORD(MAC_REVISION, "MAC Revision"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3790  	    IPW2100_ORD(RADIO_VERSION, "Radio Version"),
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3791  	    IPW2100_ORD(NIC_MANF_DATE_TIME, "MANF Date/Time STAMP"),
ee8e365aa6395e7 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-09-14  3792  	    IPW2100_ORD(UCODE_VERSION, "Ucode Version"),};
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3793  
edfc43f2ec542c1 drivers/net/wireless/ipw2100.c               Andrew Morton    2005-06-20 @3794  static ssize_t show_registers(struct device *d, struct device_attribute *attr,
edfc43f2ec542c1 drivers/net/wireless/ipw2100.c               Andrew Morton    2005-06-20  3795  			      char *buf)
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3796  {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3797  	int i;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3798  	struct ipw2100_priv *priv = dev_get_drvdata(d);
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3799  	struct net_device *dev = priv->net_dev;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3800  	char *out = buf;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3801  	u32 val = 0;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3802  
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3803  	out += sprintf(out, "%30s [Address ] : Hex\n", "Register");
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3804  
22d574324939d62 drivers/net/wireless/ipw2100.c               Ahmed S. Darwish 2007-02-05  3805  	for (i = 0; i < ARRAY_SIZE(hw_data); i++) {
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3806  		read_register(dev, hw_data[i].addr, &val);
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3807  		out += sprintf(out, "%30s [%08X] : %08X\n",
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3808  			       hw_data[i].name, hw_data[i].addr, val);
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3809  	}
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3810  
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3811  	return out - buf;
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3812  }
2c86c275015c880 drivers/net/wireless/ipw2100.c               James Ketrenos   2005-03-23  3813  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 73539 bytes --]

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

* Re: [PATCH v2 1/3] kgdb: Honour the kprobe blocklist when setting breakpoints
  2020-07-16 20:48   ` kernel test robot
@ 2020-07-17  8:13     ` Masami Hiramatsu
  2020-07-17  8:42       ` [PATCH] kprobes: Remove show_registers() function prototype Masami Hiramatsu
  0 siblings, 1 reply; 17+ messages in thread
From: Masami Hiramatsu @ 2020-07-17  8:13 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2810 bytes --]

On Fri, 17 Jul 2020 04:48:52 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi Daniel,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on kgdb/kgdb-next pmladek/for-next v5.8-rc5 next-20200716]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Daniel-Thompson/kgdb-Honour-the-kprobe-blacklist-when-setting-breakpoints/20200716-232506
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f8456690ba8eb18ea4714e68554e242a04f65cff
> config: arm64-allyesconfig (attached as .config)
> compiler: aarch64-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/net/wireless/intel/ipw2x00/ipw2100.c:3794:16: error: conflicting types for 'show_registers'
>     3794 | static ssize_t show_registers(struct device *d, struct device_attribute *attr,
>          |                ^~~~~~~~~~~~~~
>    In file included from include/linux/kgdb.h:19,
>                     from arch/arm64/include/asm/cacheflush.h:11,
>                     from include/linux/highmem.h:12,
>                     from include/linux/pagemap.h:11,
>                     from include/linux/blkdev.h:16,
>                     from include/linux/blk-cgroup.h:23,
>                     from include/linux/writeback.h:14,
>                     from include/linux/memcontrol.h:22,
>                     from include/net/sock.h:53,
>                     from include/linux/tcp.h:19,
>                     from drivers/net/wireless/intel/ipw2x00/ipw2100.c:144:
>    include/linux/kprobes.h:230:13: note: previous declaration of 'show_registers' was here
>      230 | extern void show_registers(struct pt_regs *regs);
>          |             ^~~~~~~~~~~~~~

It seems this function had been introduced before git, and forgot to removed from kprobes.h.

OK, commit 57da8b960b9a ("x86: Avoid double stack traces with show_regs()") renamed
show_registers() to show_regs() but forgot to remove old prototype in kprobes.h.

Anyway, we should remove it because kprobes doesn't use it anymore.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* [PATCH] kprobes: Remove show_registers() function prototype
  2020-07-17  8:13     ` Masami Hiramatsu
@ 2020-07-17  8:42       ` Masami Hiramatsu
  0 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2020-07-17  8:42 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

Remove show_registers() function prototype because this function
has been renamed by commit 57da8b960b9a ("x86: Avoid double stack
traces with show_regs()"), and commit 80006dbee674 ("kprobes/x86:
Remove jprobe implementation") has removed the caller in kprobes.
So this doesn't exist anymore.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/kprobes.h |    1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 45b8cdc9fad7..9be1bff4f586 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -227,7 +227,6 @@ extern int arch_prepare_kprobe(struct kprobe *p);
 extern void arch_arm_kprobe(struct kprobe *p);
 extern void arch_disarm_kprobe(struct kprobe *p);
 extern int arch_init_kprobes(void);
-extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
 extern int arch_populate_kprobe_blacklist(void);

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

* Re: [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints
  2020-07-16 15:19 [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Daniel Thompson
                   ` (2 preceding siblings ...)
  2020-07-16 15:19 ` [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions Daniel Thompson
@ 2020-07-17 13:48 ` Masami Hiramatsu
  3 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2020-07-17 13:48 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Jason Wessel, Douglas Anderson, Peter Zijlstra, sumit.garg,
	pmladek, sergey.senozhatsky, will, Masami Hiramatsu,
	kgdb-bugreport, linux-kernel, patches

Hi Daniel,

On Thu, 16 Jul 2020 16:19:40 +0100
Daniel Thompson <daniel.thompson@linaro.org> wrote:

> kgdb has traditionally adopted a no safety rails approach to breakpoint
> placement. If the debugger is commanded to place a breakpoint at an
> address then it will do so even if that breakpoint results in kgdb
> becoming inoperable.
> 
> A stop-the-world debugger with memory peek/poke intrinsically provides
> its operator with the means to hose their system in all manner of
> exciting ways (not least because stopping-the-world is already a DoS
> attack ;-) ). Nevertheless the current no safety rail approach is
> difficult to defend, especially given kprobes can provide us with plenty
> of machinery to mark the parts of the kernel where breakpointing is
> discouraged.
> 
> This patchset introduces some safety rails by using the existing kprobes
> infrastructure and ensures this will be enabled by default on
> architectures that implement kprobes. At present it does not cover
> absolutely all locations where breakpoints can cause trouble but it will
> block off several avenues, including the architecture specific parts
> that are handled by arch_within_kprobe_blacklist().

This series looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

To fix the build error with ipw2x00 driver, please feel free to
include my fix patch.

Thank you,

> 
> 
> Daniel Thompson (3):
>   kgdb: Honour the kprobe blocklist when setting breakpoints
>   kgdb: Use the kprobe blocklist to limit single stepping
>   kgdb: Add NOKPROBE labels on the trap handler functions
> 
>  include/linux/kgdb.h        | 19 +++++++++++++++++++
>  kernel/debug/debug_core.c   | 25 +++++++++++++++++++++++++
>  kernel/debug/gdbstub.c      | 10 +++++++++-
>  kernel/debug/kdb/kdb_bp.c   | 17 +++++++++++------
>  kernel/debug/kdb/kdb_main.c | 10 ++++++++--
>  lib/Kconfig.kgdb            | 14 ++++++++++++++
>  6 files changed, 86 insertions(+), 9 deletions(-)
> 
> --
> 2.25.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: [PATCH v2 1/3] kgdb: Honour the kprobe blocklist when setting breakpoints
  2020-07-16 15:19 ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist " Daniel Thompson
  2020-07-16 20:48   ` kernel test robot
@ 2020-07-17 22:39   ` Doug Anderson
  1 sibling, 0 replies; 17+ messages in thread
From: Doug Anderson @ 2020-07-17 22:39 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

Hi,

On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> Currently kgdb has absolutely no safety rails in place to discourage or
> prevent a user from placing a breakpoint in dangerous places such as
> the debugger's own trap entry/exit and other places where it is not safe
> to take synchronous traps.
>
> Introduce a new config symbol KGDB_HONOUR_BLOCKLIST and modify the
> default implementation of kgdb_validate_break_address() so that we use
> the kprobe blocklist to prohibit instrumentation of critical functions
> if the config symbol is set. The config symbol dependencies are set to
> ensure that the blocklist will be enabled by default if we enable KGDB
> and are compiling for an architecture where we HAVE_KPROBES.
>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  include/linux/kgdb.h      | 18 ++++++++++++++++++
>  kernel/debug/debug_core.c |  4 ++++
>  kernel/debug/kdb/kdb_bp.c |  9 +++++++++
>  lib/Kconfig.kgdb          | 14 ++++++++++++++
>  4 files changed, 45 insertions(+)

Seems reasonable to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping
  2020-07-16 15:19 ` [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping Daniel Thompson
@ 2020-07-17 22:39   ` Doug Anderson
  2020-07-20  8:07     ` Daniel Thompson
  0 siblings, 1 reply; 17+ messages in thread
From: Doug Anderson @ 2020-07-17 22:39 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

Hi,

On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> If we are running in a part of the kernel that dislikes breakpoint
> debugging then it is very unlikely to be safe to single step. Add
> some safety rails to prevent stepping through anything on the kprobe
> blocklist.
>
> As part of this kdb_ss() will no longer set the DOING_SS flags when it
> requests a step. This is safe because this flag is already redundant,
> returning KDB_CMD_SS is all that is needed to request a step (and this
> saves us from having to unset the flag if the safety check fails).
>
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  include/linux/kgdb.h        |  1 +
>  kernel/debug/debug_core.c   | 13 +++++++++++++
>  kernel/debug/gdbstub.c      | 10 +++++++++-
>  kernel/debug/kdb/kdb_bp.c   |  8 ++------
>  kernel/debug/kdb/kdb_main.c | 10 ++++++++--
>  5 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index 7caba4604edc..aefe823998cb 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -214,6 +214,7 @@ extern void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc);
>
>  /* Optional functions. */
>  extern int kgdb_validate_break_address(unsigned long addr);
> +extern int kgdb_validate_single_step_address(unsigned long addr);
>  extern int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt);
>  extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
>
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 133a361578dc..4b59bcc90c5d 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -208,6 +208,19 @@ int __weak kgdb_validate_break_address(unsigned long addr)
>         return err;
>  }
>
> +int __weak kgdb_validate_single_step_address(unsigned long addr)
> +{
> +       /*
> +        * Disallow stepping when we are executing code that is marked
> +        * as unsuitable for breakpointing... stepping won't be safe
> +        * either!
> +        */
> +       if (kgdb_within_blocklist(addr))
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
>  unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs)
>  {
>         return instruction_pointer(regs);
> diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> index 61774aec46b4..f1c88007cc2b 100644
> --- a/kernel/debug/gdbstub.c
> +++ b/kernel/debug/gdbstub.c
> @@ -1041,8 +1041,16 @@ int gdb_serial_stub(struct kgdb_state *ks)
>                         if (tmp == 0)
>                                 break;
>                         /* Fall through - on tmp < 0 */
> -               case 'c': /* Continue packet */
>                 case 's': /* Single step packet */
> +                       error = kgdb_validate_single_step_address(
> +                                       kgdb_arch_pc(ks->ex_vector,
> +                                                    ks->linux_regs));

I'm a little confused.  Isn't this like saying "if
(i_am_standing_in_acid) dont_step_into_acid"?

Specifically you're checking the _current_ PC to see if it's in the
blocklist, right?  ...but you've already (effectively) dropped into
the debugger at that location, so if it really was a problem wouldn't
we already be in trouble?

What you really want (I think?) is to know if the instruction that
you're stepping into is in the blocklist, right?  ...but you can't
know that because it requires a full instruction emulator (that's why
CPUs have "single step mode").

I guess you get a marginal benefit if someone manually set their
instruction pointer to be an address in the middle of a blocklisted
function and then trying to step, but I'm not sure that's really
something we need to add code for?

It feels like the right solution is that the architecture-specific
single-step code should simply consider a single-step through a
blocklisted area to be a step through one giant instruction.


> +                       if (error != 0) {
> +                               error_packet(remcom_out_buffer, error);
> +                               break;
> +                       }
> +                       fallthrough;
> +               case 'c': /* Continue packet */
>                         if (kgdb_contthread && kgdb_contthread != current) {
>                                 /* Can't switch threads in kgdb */
>                                 error_packet(remcom_out_buffer, -EINVAL);
> diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
> index ec4940146612..4853c413f579 100644
> --- a/kernel/debug/kdb/kdb_bp.c
> +++ b/kernel/debug/kdb/kdb_bp.c
> @@ -507,18 +507,14 @@ static int kdb_bc(int argc, const char **argv)
>   *     None.
>   * Remarks:
>   *
> - *     Set the arch specific option to trigger a debug trap after the next
> - *     instruction.
> + *     KDB_CMD_SS is a command that our caller acts on to effect the step.
>   */
>
>  static int kdb_ss(int argc, const char **argv)
>  {
>         if (argc != 0)
>                 return KDB_ARGCOUNT;
> -       /*
> -        * Set trace flag and go.
> -        */
> -       KDB_STATE_SET(DOING_SS);
> +
>         return KDB_CMD_SS;
>  }
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 5c7949061671..cd40bf780b93 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1189,7 +1189,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
>                      kdb_dbtrap_t db_result)
>  {
>         char *cmdbuf;
> -       int diag;
> +       int diag, res;
>         struct task_struct *kdb_current =
>                 kdb_curr_task(raw_smp_processor_id());
>
> @@ -1346,10 +1346,16 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
>                 }
>                 if (diag == KDB_CMD_GO
>                  || diag == KDB_CMD_CPU
> -                || diag == KDB_CMD_SS
>                  || diag == KDB_CMD_KGDB)
>                         break;
>
> +               if (diag == KDB_CMD_SS) {
> +                       res = kgdb_validate_single_step_address(instruction_pointer(regs));

Is it legit to use instruction_pointer() directly?  Should you be
calling kgdb_arch_pc()  ...or does that just account for having just
hit a breakpoint?

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

* Re: [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions
  2020-07-16 15:19 ` [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions Daniel Thompson
@ 2020-07-17 22:39   ` Doug Anderson
  2020-07-20  8:13     ` Daniel Thompson
  0 siblings, 1 reply; 17+ messages in thread
From: Doug Anderson @ 2020-07-17 22:39 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

Hi,

On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> Currently kgdb honours the kprobe blocklist but doesn't place its own
> trap handling code on the list. Add labels to discourage attempting to
> use kgdb to debug itself.
>
> These changes do not make it impossible to provoke recursive trapping
> since they do not cover all the calls that can be made on kgdb's entry
> logic. However going much further whilst we are sharing the kprobe
> blocklist risks reducing the capabilities of kprobe and this would be a
> bad trade off (especially so given kgdb's users are currently conditioned
> to avoid recursive traps).
>
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  kernel/debug/debug_core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

I could just be missing something, but...

I understand not adding "NOKPROBE_SYMBOL" to generic kernel functions
that kgdb happens to call, but I'm not quite sure I understand why all
of the kdb / kgdb code itself shouldn't be in the blocklist.  I
certainly don't object to the functions you added to the blocklist, I
guess I'm must trying to understand why it's a bad idea to add more or
how you came up with the list of functions that you did.

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

* Re: [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping
  2020-07-17 22:39   ` Doug Anderson
@ 2020-07-20  8:07     ` Daniel Thompson
  2020-07-21 21:04       ` Doug Anderson
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Thompson @ 2020-07-20  8:07 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

On Fri, Jul 17, 2020 at 03:39:51PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > If we are running in a part of the kernel that dislikes breakpoint
> > debugging then it is very unlikely to be safe to single step. Add
> > some safety rails to prevent stepping through anything on the kprobe
> > blocklist.
> >
> > As part of this kdb_ss() will no longer set the DOING_SS flags when it
> > requests a step. This is safe because this flag is already redundant,
> > returning KDB_CMD_SS is all that is needed to request a step (and this
> > saves us from having to unset the flag if the safety check fails).
> >
> > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > ---
> >  include/linux/kgdb.h        |  1 +
> >  kernel/debug/debug_core.c   | 13 +++++++++++++
> >  kernel/debug/gdbstub.c      | 10 +++++++++-
> >  kernel/debug/kdb/kdb_bp.c   |  8 ++------
> >  kernel/debug/kdb/kdb_main.c | 10 ++++++++--
> >  5 files changed, 33 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> > index 7caba4604edc..aefe823998cb 100644
> > --- a/include/linux/kgdb.h
> > +++ b/include/linux/kgdb.h
> > @@ -214,6 +214,7 @@ extern void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc);
> >
> >  /* Optional functions. */
> >  extern int kgdb_validate_break_address(unsigned long addr);
> > +extern int kgdb_validate_single_step_address(unsigned long addr);
> >  extern int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt);
> >  extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
> >
> > diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> > index 133a361578dc..4b59bcc90c5d 100644
> > --- a/kernel/debug/debug_core.c
> > +++ b/kernel/debug/debug_core.c
> > @@ -208,6 +208,19 @@ int __weak kgdb_validate_break_address(unsigned long addr)
> >         return err;
> >  }
> >
> > +int __weak kgdb_validate_single_step_address(unsigned long addr)
> > +{
> > +       /*
> > +        * Disallow stepping when we are executing code that is marked
> > +        * as unsuitable for breakpointing... stepping won't be safe
> > +        * either!
> > +        */
> > +       if (kgdb_within_blocklist(addr))
> > +               return -EINVAL;
> > +
> > +       return 0;
> > +}
> > +
> >  unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs)
> >  {
> >         return instruction_pointer(regs);
> > diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> > index 61774aec46b4..f1c88007cc2b 100644
> > --- a/kernel/debug/gdbstub.c
> > +++ b/kernel/debug/gdbstub.c
> > @@ -1041,8 +1041,16 @@ int gdb_serial_stub(struct kgdb_state *ks)
> >                         if (tmp == 0)
> >                                 break;
> >                         /* Fall through - on tmp < 0 */
> > -               case 'c': /* Continue packet */
> >                 case 's': /* Single step packet */
> > +                       error = kgdb_validate_single_step_address(
> > +                                       kgdb_arch_pc(ks->ex_vector,
> > +                                                    ks->linux_regs));
> 
> I'm a little confused.  Isn't this like saying "if
> (i_am_standing_in_acid) dont_step_into_acid"?

I describe it more as:

    if (we_know_there_is_acid_nearby)
        dont_step_forward

It is possible we are currently stepping in acid but it is also possible
(and reasonably likely) that we haven't stepped in it yet but will do so
soon.


> Specifically you're checking the _current_ PC to see if it's in the
> blocklist, right?  ...but you've already (effectively) dropped into
> the debugger at that location, so if it really was a problem wouldn't
> we already be in trouble?

The basic use case is where someone is stepping and we reach a PC that
would be blocked for a breakpoint. This will typically be due (although
I think it does generalize) to a function call and the safety rail will
be reached after we have jumped to the blocked function but before we
actually execute any instructions within it.

Or putting it another way, there is no reason to worry if we start
somewhere "safe" and start stepping towards something on the blocklist.
We won't melt our shoes!

There are more complex cases when we drop into the debugger in the
middle of blocked code with a not-breakpoint-or-step trap. You're right
that we'd been in touble and the debugger it probably a bit fragile.
However that certainly doesn't mean blocking stepping at this point
is a bad thing!


> What you really want (I think?) is to know if the instruction that
> you're stepping into is in the blocklist, right?  ...but you can't
> know that because it requires a full instruction emulator (that's why
> CPUs have "single step mode").

As above, I don't think this is needed but if there was an architecture
that did then it can override the default implementation if it wanted
to.

 
> I guess you get a marginal benefit if someone manually set their
> instruction pointer to be an address in the middle of a blocklisted
> function and then trying to step, but I'm not sure that's really
> something we need to add code for?

Perhaps off-topic given this isn't why we add the satefy rails but...

I think people who directly set PC should be regarded as very
sophisticated users (and therefore do not need safety rails) so I have
little interest in honouring the blocklist for direct writes to the
PC. More generally sophisticated users should be able to find
KGDB_HONOUR_BLOCKLIST pretty quickly if they need to!


> It feels like the right solution is that the architecture-specific
> single-step code should simply consider a single-step through a
> blocklisted area to be a step through one giant instruction.

For kgdb this feature is already implemented (next or finish).


> > +                       if (error != 0) {
> > +                               error_packet(remcom_out_buffer, error);
> > +                               break;
> > +                       }
> > +                       fallthrough;
> > +               case 'c': /* Continue packet */
> >                         if (kgdb_contthread && kgdb_contthread != current) {
> >                                 /* Can't switch threads in kgdb */
> >                                 error_packet(remcom_out_buffer, -EINVAL);
> > diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
> > index ec4940146612..4853c413f579 100644
> > --- a/kernel/debug/kdb/kdb_bp.c
> > +++ b/kernel/debug/kdb/kdb_bp.c
> > @@ -507,18 +507,14 @@ static int kdb_bc(int argc, const char **argv)
> >   *     None.
> >   * Remarks:
> >   *
> > - *     Set the arch specific option to trigger a debug trap after the next
> > - *     instruction.
> > + *     KDB_CMD_SS is a command that our caller acts on to effect the step.
> >   */
> >
> >  static int kdb_ss(int argc, const char **argv)
> >  {
> >         if (argc != 0)
> >                 return KDB_ARGCOUNT;
> > -       /*
> > -        * Set trace flag and go.
> > -        */
> > -       KDB_STATE_SET(DOING_SS);
> > +
> >         return KDB_CMD_SS;
> >  }
> >
> > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> > index 5c7949061671..cd40bf780b93 100644
> > --- a/kernel/debug/kdb/kdb_main.c
> > +++ b/kernel/debug/kdb/kdb_main.c
> > @@ -1189,7 +1189,7 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
> >                      kdb_dbtrap_t db_result)
> >  {
> >         char *cmdbuf;
> > -       int diag;
> > +       int diag, res;
> >         struct task_struct *kdb_current =
> >                 kdb_curr_task(raw_smp_processor_id());
> >
> > @@ -1346,10 +1346,16 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
> >                 }
> >                 if (diag == KDB_CMD_GO
> >                  || diag == KDB_CMD_CPU
> > -                || diag == KDB_CMD_SS
> >                  || diag == KDB_CMD_KGDB)
> >                         break;
> >
> > +               if (diag == KDB_CMD_SS) {
> > +                       res = kgdb_validate_single_step_address(instruction_pointer(regs));
> 
> Is it legit to use instruction_pointer() directly?  Should you be
> calling kgdb_arch_pc()  ...or does that just account for having just
> hit a breakpoint?

I decided between kgdb_arch_pc() and instruction_pointer() based on the
usage of regs in the rest of this file (which is exclusively
instruction_pointer() ). I didn't want the lookup to mismatch what the
user has been told in the console.

On the other hand, I did cross my mind that every PC lookup could be
broken and I made a note for the future...


Daniel.

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

* Re: [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions
  2020-07-17 22:39   ` Doug Anderson
@ 2020-07-20  8:13     ` Daniel Thompson
  2020-07-21 21:22       ` Doug Anderson
  0 siblings, 1 reply; 17+ messages in thread
From: Daniel Thompson @ 2020-07-20  8:13 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

On Fri, Jul 17, 2020 at 03:39:58PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > Currently kgdb honours the kprobe blocklist but doesn't place its own
> > trap handling code on the list. Add labels to discourage attempting to
> > use kgdb to debug itself.
> >
> > These changes do not make it impossible to provoke recursive trapping
> > since they do not cover all the calls that can be made on kgdb's entry
> > logic. However going much further whilst we are sharing the kprobe
> > blocklist risks reducing the capabilities of kprobe and this would be a
> > bad trade off (especially so given kgdb's users are currently conditioned
> > to avoid recursive traps).
> >
> > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > ---
> >  kernel/debug/debug_core.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> 
> I could just be missing something, but...
> 
> I understand not adding "NOKPROBE_SYMBOL" to generic kernel functions
> that kgdb happens to call, but I'm not quite sure I understand why all
> of the kdb / kgdb code itself shouldn't be in the blocklist.  I
> certainly don't object to the functions you added to the blocklist, I
> guess I'm must trying to understand why it's a bad idea to add more or
> how you came up with the list of functions that you did.

Relatively early in the trap handler execution (just after we bring the
other CPUs to a halt) all breakpoints are replaced with the original
opcodes. Therefore I only marked up functions that run between the trap
firing and the breakpoints being removed (and also between the
breakpoints being reinstated and trap exit).


Daniel.

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

* Re: [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping
  2020-07-20  8:07     ` Daniel Thompson
@ 2020-07-21 21:04       ` Doug Anderson
  2020-09-04 15:22         ` Daniel Thompson
  0 siblings, 1 reply; 17+ messages in thread
From: Doug Anderson @ 2020-07-21 21:04 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

Hi,

On Mon, Jul 20, 2020 at 1:08 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Fri, Jul 17, 2020 at 03:39:51PM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
> > <daniel.thompson@linaro.org> wrote:
> > >
> > > If we are running in a part of the kernel that dislikes breakpoint
> > > debugging then it is very unlikely to be safe to single step. Add
> > > some safety rails to prevent stepping through anything on the kprobe
> > > blocklist.
> > >
> > > As part of this kdb_ss() will no longer set the DOING_SS flags when it
> > > requests a step. This is safe because this flag is already redundant,
> > > returning KDB_CMD_SS is all that is needed to request a step (and this
> > > saves us from having to unset the flag if the safety check fails).
> > >
> > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > > ---
> > >  include/linux/kgdb.h        |  1 +
> > >  kernel/debug/debug_core.c   | 13 +++++++++++++
> > >  kernel/debug/gdbstub.c      | 10 +++++++++-
> > >  kernel/debug/kdb/kdb_bp.c   |  8 ++------
> > >  kernel/debug/kdb/kdb_main.c | 10 ++++++++--
> > >  5 files changed, 33 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> > > index 7caba4604edc..aefe823998cb 100644
> > > --- a/include/linux/kgdb.h
> > > +++ b/include/linux/kgdb.h
> > > @@ -214,6 +214,7 @@ extern void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc);
> > >
> > >  /* Optional functions. */
> > >  extern int kgdb_validate_break_address(unsigned long addr);
> > > +extern int kgdb_validate_single_step_address(unsigned long addr);
> > >  extern int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt);
> > >  extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
> > >
> > > diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> > > index 133a361578dc..4b59bcc90c5d 100644
> > > --- a/kernel/debug/debug_core.c
> > > +++ b/kernel/debug/debug_core.c
> > > @@ -208,6 +208,19 @@ int __weak kgdb_validate_break_address(unsigned long addr)
> > >         return err;
> > >  }
> > >
> > > +int __weak kgdb_validate_single_step_address(unsigned long addr)
> > > +{
> > > +       /*
> > > +        * Disallow stepping when we are executing code that is marked
> > > +        * as unsuitable for breakpointing... stepping won't be safe
> > > +        * either!
> > > +        */
> > > +       if (kgdb_within_blocklist(addr))
> > > +               return -EINVAL;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs)
> > >  {
> > >         return instruction_pointer(regs);
> > > diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> > > index 61774aec46b4..f1c88007cc2b 100644
> > > --- a/kernel/debug/gdbstub.c
> > > +++ b/kernel/debug/gdbstub.c
> > > @@ -1041,8 +1041,16 @@ int gdb_serial_stub(struct kgdb_state *ks)
> > >                         if (tmp == 0)
> > >                                 break;
> > >                         /* Fall through - on tmp < 0 */
> > > -               case 'c': /* Continue packet */
> > >                 case 's': /* Single step packet */
> > > +                       error = kgdb_validate_single_step_address(
> > > +                                       kgdb_arch_pc(ks->ex_vector,
> > > +                                                    ks->linux_regs));
> >
> > I'm a little confused.  Isn't this like saying "if
> > (i_am_standing_in_acid) dont_step_into_acid"?
>
> I describe it more as:
>
>     if (we_know_there_is_acid_nearby)
>         dont_step_forward
>
> It is possible we are currently stepping in acid but it is also possible
> (and reasonably likely) that we haven't stepped in it yet but will do so
> soon.
>
>
> > Specifically you're checking the _current_ PC to see if it's in the
> > blocklist, right?  ...but you've already (effectively) dropped into
> > the debugger at that location, so if it really was a problem wouldn't
> > we already be in trouble?
>
> The basic use case is where someone is stepping and we reach a PC that
> would be blocked for a breakpoint. This will typically be due (although
> I think it does generalize) to a function call and the safety rail will
> be reached after we have jumped to the blocked function but before we
> actually execute any instructions within it.
>
> Or putting it another way, there is no reason to worry if we start
> somewhere "safe" and start stepping towards something on the blocklist.
> We won't melt our shoes!

I guess I still don't totally get it.  So let's say we have:

void dont_trace_this(...)
{
  thing_not_to_trace_1();
  thing_not_to_trace_2();
  don_t_trace = this;
}
NOKPROBE_SYMBOL(dont_trace_this);

void trace_me()
{
  sing();
  dance();
  dont_trace_this();
  party();
}

So presumably the dont_trace_this() function is marked as
NOKPROBE_SYMBOL because it's called by the kprobe handling code or by
kgdb, right?  So if we had a breakpoint there then we'd just have
infinite recursion.  Thus we want to prevent putting breakpoints
anywhere in this function.  Even though dont_trace_this() is also
called from the trace_me() function it doesn't matter--we still can't
put breakpoints in it because it would cause problems with the
debugger.

Now, I guess the question is: why exactly do we need to prevent single
stepping in dont_trace_this().  In the case above where
dont_trace_this() is called from trace_me() it would actually be OK to
single step it, right?  ...unless this is on a CPU that doesn't have a
"single step mode" and has to implement stepping by breakpoints, of
course.

...but maybe I'm confused and there is a reason that we shouldn't
allow single stepping into dont_trace_this() when called from
trace_me().  If that is the case, I'm wondering why it's OK to step
and stop on the first instruction of the function but it's not OK to
step and stop through the other instructions in the function.

-Doug

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

* Re: [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions
  2020-07-20  8:13     ` Daniel Thompson
@ 2020-07-21 21:22       ` Doug Anderson
  2020-09-04 15:40         ` Daniel Thompson
  0 siblings, 1 reply; 17+ messages in thread
From: Doug Anderson @ 2020-07-21 21:22 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

Hi,

On Mon, Jul 20, 2020 at 1:13 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Fri, Jul 17, 2020 at 03:39:58PM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
> > <daniel.thompson@linaro.org> wrote:
> > >
> > > Currently kgdb honours the kprobe blocklist but doesn't place its own
> > > trap handling code on the list. Add labels to discourage attempting to
> > > use kgdb to debug itself.
> > >
> > > These changes do not make it impossible to provoke recursive trapping
> > > since they do not cover all the calls that can be made on kgdb's entry
> > > logic. However going much further whilst we are sharing the kprobe
> > > blocklist risks reducing the capabilities of kprobe and this would be a
> > > bad trade off (especially so given kgdb's users are currently conditioned
> > > to avoid recursive traps).
> > >
> > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > > ---
> > >  kernel/debug/debug_core.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> >
> > I could just be missing something, but...
> >
> > I understand not adding "NOKPROBE_SYMBOL" to generic kernel functions
> > that kgdb happens to call, but I'm not quite sure I understand why all
> > of the kdb / kgdb code itself shouldn't be in the blocklist.  I
> > certainly don't object to the functions you added to the blocklist, I
> > guess I'm must trying to understand why it's a bad idea to add more or
> > how you came up with the list of functions that you did.
>
> Relatively early in the trap handler execution (just after we bring the
> other CPUs to a halt) all breakpoints are replaced with the original
> opcodes. Therefore I only marked up functions that run between the trap
> firing and the breakpoints being removed (and also between the
> breakpoints being reinstated and trap exit).

Ah, OK!  Could that be added to the commit message?

Also, shouldn't you mark kgdb_arch_set_breakpoint()?  What about
dbg_activate_sw_breakpoints()?  I haven't gone and extensively
searched, but those two jump out to me as ones that were missed.

I suppose that means that if someone tried to set a breakpoint on a
kgdb function that wasn't one of the ones that you listed then the
system would happily report that the breakpoint has been set (no error
given) but that the breakpoint would just have no effect?  It wouldn't
crash (which is good), it just wouldn't detect that the breakpoint was
useless.  However, if these were in the NOKPROBE_SYMBOL then you'd get
a nice error message.  Is there no way we could use a linker script to
just mark everything using a linker script or somesuch?

-Doug

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

* Re: [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping
  2020-07-21 21:04       ` Doug Anderson
@ 2020-09-04 15:22         ` Daniel Thompson
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Thompson @ 2020-09-04 15:22 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

On Tue, Jul 21, 2020 at 02:04:45PM -0700, Doug Anderson wrote:
> On Mon, Jul 20, 2020 at 1:08 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> > On Fri, Jul 17, 2020 at 03:39:51PM -0700, Doug Anderson wrote:
> > > On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
> > > <daniel.thompson@linaro.org> wrote:
> > > >
> > > > If we are running in a part of the kernel that dislikes breakpoint
> > > > debugging then it is very unlikely to be safe to single step. Add
> > > > some safety rails to prevent stepping through anything on the kprobe
> > > > blocklist.
> > > >
> > > > As part of this kdb_ss() will no longer set the DOING_SS flags when it
> > > > requests a step. This is safe because this flag is already redundant,
> > > > returning KDB_CMD_SS is all that is needed to request a step (and this
> > > > saves us from having to unset the flag if the safety check fails).
> > > >
> > > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > > > ---
> > > >  include/linux/kgdb.h        |  1 +
> > > >  kernel/debug/debug_core.c   | 13 +++++++++++++
> > > >  kernel/debug/gdbstub.c      | 10 +++++++++-
> > > >  kernel/debug/kdb/kdb_bp.c   |  8 ++------
> > > >  kernel/debug/kdb/kdb_main.c | 10 ++++++++--
> > > >  5 files changed, 33 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> > > > index 7caba4604edc..aefe823998cb 100644
> > > > --- a/include/linux/kgdb.h
> > > > +++ b/include/linux/kgdb.h
> > > > @@ -214,6 +214,7 @@ extern void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc);
> > > >
> > > >  /* Optional functions. */
> > > >  extern int kgdb_validate_break_address(unsigned long addr);
> > > > +extern int kgdb_validate_single_step_address(unsigned long addr);
> > > >  extern int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt);
> > > >  extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
> > > >
> > > > diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> > > > index 133a361578dc..4b59bcc90c5d 100644
> > > > --- a/kernel/debug/debug_core.c
> > > > +++ b/kernel/debug/debug_core.c
> > > > @@ -208,6 +208,19 @@ int __weak kgdb_validate_break_address(unsigned long addr)
> > > >         return err;
> > > >  }
> > > >
> > > > +int __weak kgdb_validate_single_step_address(unsigned long addr)
> > > > +{
> > > > +       /*
> > > > +        * Disallow stepping when we are executing code that is marked
> > > > +        * as unsuitable for breakpointing... stepping won't be safe
> > > > +        * either!
> > > > +        */
> > > > +       if (kgdb_within_blocklist(addr))
> > > > +               return -EINVAL;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > >  unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs)
> > > >  {
> > > >         return instruction_pointer(regs);
> > > > diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> > > > index 61774aec46b4..f1c88007cc2b 100644
> > > > --- a/kernel/debug/gdbstub.c
> > > > +++ b/kernel/debug/gdbstub.c
> > > > @@ -1041,8 +1041,16 @@ int gdb_serial_stub(struct kgdb_state *ks)
> > > >                         if (tmp == 0)
> > > >                                 break;
> > > >                         /* Fall through - on tmp < 0 */
> > > > -               case 'c': /* Continue packet */
> > > >                 case 's': /* Single step packet */
> > > > +                       error = kgdb_validate_single_step_address(
> > > > +                                       kgdb_arch_pc(ks->ex_vector,
> > > > +                                                    ks->linux_regs));
> > >
> > > I'm a little confused.  Isn't this like saying "if
> > > (i_am_standing_in_acid) dont_step_into_acid"?
> >
> > I describe it more as:
> >
> >     if (we_know_there_is_acid_nearby)
> >         dont_step_forward
> >
> > It is possible we are currently stepping in acid but it is also possible
> > (and reasonably likely) that we haven't stepped in it yet but will do so
> > soon.
> >
> >
> > > Specifically you're checking the _current_ PC to see if it's in the
> > > blocklist, right?  ...but you've already (effectively) dropped into
> > > the debugger at that location, so if it really was a problem wouldn't
> > > we already be in trouble?
> >
> > The basic use case is where someone is stepping and we reach a PC that
> > would be blocked for a breakpoint. This will typically be due (although
> > I think it does generalize) to a function call and the safety rail will
> > be reached after we have jumped to the blocked function but before we
> > actually execute any instructions within it.
> >
> > Or putting it another way, there is no reason to worry if we start
> > somewhere "safe" and start stepping towards something on the blocklist.
> > We won't melt our shoes!
> 
> I guess I still don't totally get it.  So let's say we have:
> 
> void dont_trace_this(...)
> {
>   thing_not_to_trace_1();
>   thing_not_to_trace_2();
>   don_t_trace = this;
> }
> NOKPROBE_SYMBOL(dont_trace_this);
> 
> void trace_me()
> {
>   sing();
>   dance();
>   dont_trace_this();
>   party();
> }
> 
> So presumably the dont_trace_this() function is marked as
> NOKPROBE_SYMBOL because it's called by the kprobe handling code or by
> kgdb, right?  So if we had a breakpoint there then we'd just have
> infinite recursion.  Thus we want to prevent putting breakpoints
> anywhere in this function.  Even though dont_trace_this() is also
> called from the trace_me() function it doesn't matter--we still can't
> put breakpoints in it because it would cause problems with the
> debugger.
> 
> Now, I guess the question is: why exactly do we need to prevent single
> stepping in dont_trace_this().  In the case above where
> dont_trace_this() is called from trace_me() it would actually be OK to
> single step it, right?  ...unless this is on a CPU that doesn't have a
> "single step mode" and has to implement stepping by breakpoints, of
> course.

I think you are persuading me.

Although I can think of plenty of places where it isn't safe to step I'm
struggling to think of any way for us to end up stopped in the debugger
in those places and certainly now without setting the catastrophic
(which is the only safety rail currently extant).

That means I don't think I can put up a strong enough case that this
patch is better than doing nothing!

I'll drop it for now.

> ...but maybe I'm confused

I think on the whole you've expressed things more lucidly than I have!
Nevertheless...

> and there is a reason that we shouldn't
> allow single stepping into dont_trace_this() when called from
> trace_me().  If that is the case, I'm wondering why it's OK to step
> and stop on the first instruction of the function but it's not OK to
> step and stop through the other instructions in the function.

... when we stop on the first instruction of a function then we have not
actually executed any part of it. In other words we haven't executed
anything on the blocklist.

Of course the whole issue is moot for now.


Daniel.

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

* Re: [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions
  2020-07-21 21:22       ` Doug Anderson
@ 2020-09-04 15:40         ` Daniel Thompson
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Thompson @ 2020-09-04 15:40 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Jason Wessel, Peter Zijlstra, Sumit Garg, Petr Mladek,
	Sergey Senozhatsky, Will Deacon, Masami Hiramatsu,
	kgdb-bugreport, LKML, Patch Tracking

On Tue, Jul 21, 2020 at 02:22:08PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 20, 2020 at 1:13 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Fri, Jul 17, 2020 at 03:39:58PM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Thu, Jul 16, 2020 at 8:20 AM Daniel Thompson
> > > <daniel.thompson@linaro.org> wrote:
> > > >
> > > > Currently kgdb honours the kprobe blocklist but doesn't place its own
> > > > trap handling code on the list. Add labels to discourage attempting to
> > > > use kgdb to debug itself.
> > > >
> > > > These changes do not make it impossible to provoke recursive trapping
> > > > since they do not cover all the calls that can be made on kgdb's entry
> > > > logic. However going much further whilst we are sharing the kprobe
> > > > blocklist risks reducing the capabilities of kprobe and this would be a
> > > > bad trade off (especially so given kgdb's users are currently conditioned
> > > > to avoid recursive traps).
> > > >
> > > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > > > ---
> > > >  kernel/debug/debug_core.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > >
> > > I could just be missing something, but...
> > >
> > > I understand not adding "NOKPROBE_SYMBOL" to generic kernel functions
> > > that kgdb happens to call, but I'm not quite sure I understand why all
> > > of the kdb / kgdb code itself shouldn't be in the blocklist.  I
> > > certainly don't object to the functions you added to the blocklist, I
> > > guess I'm must trying to understand why it's a bad idea to add more or
> > > how you came up with the list of functions that you did.
> >
> > Relatively early in the trap handler execution (just after we bring the
> > other CPUs to a halt) all breakpoints are replaced with the original
> > opcodes. Therefore I only marked up functions that run between the trap
> > firing and the breakpoints being removed (and also between the
> > breakpoints being reinstated and trap exit).
> 
> Ah, OK!  Could that be added to the commit message?

Will do.
 
> Also, shouldn't you mark kgdb_arch_set_breakpoint()?  What about
> dbg_activate_sw_breakpoints()?  I haven't gone and extensively
> searched, but those two jump out to me as ones that were missed.

Agree. I think I over-focusses on the entry path. I will review the
exit path more closely.

> I suppose that means that if someone tried to set a breakpoint on a
> kgdb function that wasn't one of the ones that you listed then the
> system would happily report that the breakpoint has been set (no error
> given) but that the breakpoint would just have no effect?  It wouldn't
> crash (which is good), it just wouldn't detect that the breakpoint was
> useless.

Assuming the kgdb function is used exclusively from the trap handler
then this is correct.

> However, if these were in the NOKPROBE_SYMBOL then you'd get
> a nice error message.  Is there no way we could use a linker script to
> just mark everything using a linker script or somesuch?

You'd still get odd effects with library functions that are used inside
and outside the debugger (which can be breakpointed but don't trigger
inside kgdb). Arguably the effect is clearer to users if they can see
kdb/kgdb functions behaving the same way as library functions. It's odd
but it won't promote false expectations from users.


Daniel.

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

end of thread, other threads:[~2020-09-04 15:41 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 15:19 [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Daniel Thompson
2020-07-16 15:19 ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist " Daniel Thompson
2020-07-16 20:48   ` kernel test robot
2020-07-17  8:13     ` Masami Hiramatsu
2020-07-17  8:42       ` [PATCH] kprobes: Remove show_registers() function prototype Masami Hiramatsu
2020-07-17 22:39   ` [PATCH v2 1/3] kgdb: Honour the kprobe blocklist when setting breakpoints Doug Anderson
2020-07-16 15:19 ` [PATCH v2 2/3] kgdb: Use the kprobe blocklist to limit single stepping Daniel Thompson
2020-07-17 22:39   ` Doug Anderson
2020-07-20  8:07     ` Daniel Thompson
2020-07-21 21:04       ` Doug Anderson
2020-09-04 15:22         ` Daniel Thompson
2020-07-16 15:19 ` [PATCH v2 3/3] kgdb: Add NOKPROBE labels on the trap handler functions Daniel Thompson
2020-07-17 22:39   ` Doug Anderson
2020-07-20  8:13     ` Daniel Thompson
2020-07-21 21:22       ` Doug Anderson
2020-09-04 15:40         ` Daniel Thompson
2020-07-17 13:48 ` [PATCH v2 0/3] kgdb: Honour the kprobe blacklist when setting breakpoints Masami Hiramatsu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.