* [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
* 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 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
* [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
* 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 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 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 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
* [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 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 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 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 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
* 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
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.