15 Dec, 2016

2 commits

  • kdb_trap_printk allows to pass normal printk() messages to kdb via
    vkdb_printk(). For example, it is used to get backtrace using the
    classic show_stack(), see kdb_show_stack().

    vkdb_printf() tries to avoid a potential infinite loop by disabling the
    trap. But this approach is racy, for example:

    CPU1 CPU2

    vkdb_printf()
    // assume that kdb_trap_printk == 0
    saved_trap_printk = kdb_trap_printk;
    kdb_trap_printk = 0;

    kdb_show_stack()
    kdb_trap_printk++;

    Problem1: Now, a nested printk() on CPU0 calls vkdb_printf()
    even when it should have been disabled. It will not
    cause a deadlock but...

    // using the outdated saved value: 0
    kdb_trap_printk = saved_trap_printk;

    kdb_trap_printk--;

    Problem2: Now, kdb_trap_printk == -1 and will stay like this.
    It means that all messages will get passed to kdb from
    now on.

    This patch removes the racy saved_trap_printk handling. Instead, the
    recursion is prevented by a check for the locked CPU.

    The solution is still kind of racy. A non-related printk(), from
    another process, might get trapped by vkdb_printf(). And the wanted
    printk() might not get trapped because kdb_printf_cpu is assigned. But
    this problem existed even with the original code.

    A proper solution would be to get_cpu() before setting kdb_trap_printk
    and trap messages only from this CPU. I am not sure if it is worth the
    effort, though.

    In fact, the race is very theoretical. When kdb is running any of the
    commands that use kdb_trap_printk there is a single active CPU and the
    other CPUs should be in a holding pen inside kgdb_cpu_enter().

    The only time this is violated is when there is a timeout waiting for
    the other CPUs to report to the holding pen.

    Finally, note that the situation is a bit schizophrenic. vkdb_printf()
    explicitly allows recursion but only from KDB code that calls
    kdb_printf() directly. On the other hand, the generic printk()
    recursion is not allowed because it might cause an infinite loop. This
    is why we could not hide the decision inside vkdb_printf() easily.

    Link: http://lkml.kernel.org/r/1480412276-16690-4-git-send-email-pmladek@suse.com
    Signed-off-by: Petr Mladek
    Cc: Daniel Thompson
    Cc: Jason Wessel
    Cc: Peter Zijlstra
    Cc: Sergey Senozhatsky
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Petr Mladek
     
  • kdb_event state variable is only set but never checked in the kernel
    code.

    http://www.spinics.net/lists/kdb/msg01733.html suggests that this
    variable affected WARN_CONSOLE_UNLOCKED() in the original
    implementation. But this check never went upstream.

    The semantic is unclear and racy. The value is updated after the
    kdb_printf_lock is acquired and after it is released. It should be
    symmetric at minimum. The value should be manipulated either inside or
    outside the locked area.

    Fortunately, it seems that the original function is gone and we could
    simply remove the state variable.

    Link: http://lkml.kernel.org/r/1480412276-16690-2-git-send-email-pmladek@suse.com
    Signed-off-by: Petr Mladek
    Suggested-by: Daniel Thompson
    Cc: Jason Wessel
    Cc: Peter Zijlstra
    Cc: Sergey Senozhatsky
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Petr Mladek
     

29 Jul, 2016

1 commit


20 Feb, 2015

1 commit

  • Currently when kdb traps printk messages then the raw log level prefix
    (consisting of '\001' followed by a numeral) does not get stripped off
    before the message is issued to the various I/O handlers supported by
    kdb. This causes annoying visual noise as well as causing problems
    grepping for ^. It is also a change of behaviour compared to normal usage
    of printk() usage. For example -h ends up with different output to
    that of kdb's "sr h".

    This patch addresses the problem by stripping log levels from messages
    before they are issued to the I/O handlers. printk() which can also
    act as an i/o handler in some cases is special cased; if the caller
    provided a log level then the prefix will be preserved when sent to
    printk().

    The addition of non-printable characters to the output of kdb commands is a
    regression, albeit and extremely elderly one, introduced by commit
    04d2c8c83d0e ("printk: convert the format for KERN_ to a 2 byte
    pattern"). Note also that this patch does *not* restore the original
    behaviour from v3.5. Instead it makes printk() from within a kdb command
    display the message without any prefix (i.e. like printk() normally does).

    Signed-off-by: Daniel Thompson
    Cc: Joe Perches
    Cc: stable@vger.kernel.org
    Signed-off-by: Jason Wessel

    Daniel Thompson
     

11 Nov, 2014

6 commits

  • Currently all kdb commands are enabled whenever kdb is deployed. This
    makes it difficult to deploy kdb to help debug certain types of
    systems.

    Android phones provide one example; the FIQ debugger found on some
    Android devices has a deliberately weak set of commands to allow the
    debugger to enabled very late in the production cycle.

    Certain kiosk environments offer another interesting case where an
    engineer might wish to probe the system state using passive inspection
    commands without providing sufficient power for a passer by to root it.

    Without any restrictions, obtaining the root rights via KDB is a matter of
    a few commands, and works everywhere. For example, log in as a normal
    user:

    cbou:~$ id
    uid=1001(cbou) gid=1001(cbou) groups=1001(cbou)

    Now enter KDB (for example via sysrq):

    Entering kdb (current=0xffff8800065bc740, pid 920) due to Keyboard Entry
    kdb> ps
    23 sleeping system daemon (state M) processes suppressed,
    use 'ps A' to see all.
    Task Addr Pid Parent [*] cpu State Thread Command
    0xffff8800065bc740 920 919 1 0 R 0xffff8800065bca20 *bash

    0xffff880007078000 1 0 0 0 S 0xffff8800070782e0 init
    [...snip...]
    0xffff8800065be3c0 918 1 0 0 S 0xffff8800065be6a0 getty
    0xffff8800065b9c80 919 1 0 0 S 0xffff8800065b9f60 login
    0xffff8800065bc740 920 919 1 0 R 0xffff8800065bca20 *bash

    All we need is the offset of cred pointers. We can look up the offset in
    the distro's kernel source, but it is unnecessary. We can just start
    dumping init's task_struct, until we see the process name:

    kdb> md 0xffff880007078000
    0xffff880007078000 0000000000000001 ffff88000703c000 ................
    0xffff880007078010 0040210000000002 0000000000000000 .....!@.........
    [...snip...]
    0xffff8800070782b0 ffff8800073e0580 ffff8800073e0580 ..>.......>.....
    0xffff8800070782c0 0000000074696e69 0000000000000000 init............

    ^ Here, 'init'. Creds are just above it, so the offset is 0x02b0.

    Now we set up init's creds for our non-privileged shell:

    kdb> mm 0xffff8800065bc740+0x02b0 0xffff8800073e0580
    0xffff8800065bc9f0 = 0xffff8800073e0580
    kdb> mm 0xffff8800065bc740+0x02b8 0xffff8800073e0580
    0xffff8800065bc9f8 = 0xffff8800073e0580

    And thus gaining the root:

    kdb> go
    cbou:~$ id
    uid=0(root) gid=0(root) groups=0(root)
    cbou:~$ bash
    root:~#

    p.s. No distro enables kdb by default (although, with a nice KDB-over-KMS
    feature availability, I would expect at least some would enable it), so
    it's not actually some kind of a major issue.

    Signed-off-by: Anton Vorontsov
    Signed-off-by: John Stultz
    Signed-off-by: Daniel Thompson
    Cc: Jason Wessel
    Signed-off-by: Jason Wessel

    Anton Vorontsov
     
  • This patch introduces several new flags to collect kdb commands into
    groups (later allowing them to be optionally disabled).

    This follows similar prior art to enable/disable magic sysrq
    commands.

    The commands have been categorized as follows:

    Always on: go (w/o args), env, set, help, ?, cpu (w/o args), sr,
    dmesg, disable_nmi, defcmd, summary, grephelp
    Mem read: md, mdr, mdp, mds, ef, bt (with args), per_cpu
    Mem write: mm
    Reg read: rd
    Reg write: go (with args), rm
    Inspect: bt (w/o args), btp, bta, btc, btt, ps, pid, lsmod
    Flow ctrl: bp, bl, bph, bc, be, bd, ss
    Signal: kill
    Reboot: reboot
    All: cpu, kgdb, (and all of the above), nmi_console

    Signed-off-by: Daniel Thompson
    Cc: Jason Wessel
    Signed-off-by: Jason Wessel

    Daniel Thompson
     
  • Since we now treat KDB_REPEAT_* as flags, there is no need to
    pass KDB_REPEAT_NONE. It's just the default behaviour when no
    flags are specified.

    Signed-off-by: Anton Vorontsov
    Signed-off-by: John Stultz
    Signed-off-by: Daniel Thompson
    Cc: Jason Wessel
    Signed-off-by: Jason Wessel

    Anton Vorontsov
     
  • The actual values of KDB_REPEAT_* enum values and overall logic stayed
    the same, but we now treat the values as flags.

    This makes it possible to add other flags and combine them, plus makes
    the code a lot simpler and shorter. But functionality-wise, there should
    be no changes.

    Signed-off-by: Anton Vorontsov
    Signed-off-by: John Stultz
    Signed-off-by: Daniel Thompson
    Cc: Jason Wessel
    Signed-off-by: Jason Wessel

    Anton Vorontsov
     
  • We're about to add more options for commands behaviour, so let's give
    a more generic name to the low-level kdb command registration function.

    There are just various renames, no functional changes.

    Signed-off-by: Anton Vorontsov
    Signed-off-by: John Stultz
    Signed-off-by: Daniel Thompson
    Cc: Jason Wessel
    Signed-off-by: Jason Wessel

    Anton Vorontsov
     
  • We're about to add more options for command behaviour, so let's expand
    the meaning of kdb_repeat_t.

    So far we just do various renames, there should be no functional changes.

    Signed-off-by: Anton Vorontsov
    Signed-off-by: John Stultz
    Signed-off-by: Daniel Thompson
    Cc: Jason Wessel
    Signed-off-by: Jason Wessel

    Anton Vorontsov
     

04 Oct, 2013

1 commit

  • This patch adds a kgdb_nmicallin() interface that can be used by
    external NMI handlers to call the KGDB/KDB handler. The primary
    need for this is for those types of NMI interrupts where all the
    CPUs have already received the NMI signal. Therefore no
    send_IPI(NMI) is required, and in fact it will cause a 2nd
    unhandled NMI to occur. This generates the "Dazed and Confuzed"
    messages.

    Since all the CPUs are getting the NMI at roughly the same time,
    it's not guaranteed that the first CPU that hits the NMI handler
    will manage to enter KGDB and set the dbg_master_lock before the
    slaves start entering. The new argument "send_ready" was added
    for KGDB to signal the NMI handler to release the slave CPUs for
    entry into KGDB.

    Signed-off-by: Mike Travis
    Acked-by: Jason Wessel
    Reviewed-by: Dimitri Sivanich
    Reviewed-by: Hedi Berriche
    Cc: Peter Zijlstra
    Cc: Paul Mackerras
    Cc: Arnaldo Carvalho de Melo
    Link: http://lkml.kernel.org/r/20131002151417.928886849@asylum.americas.sgi.com
    Signed-off-by: Ingo Molnar

    Mike Travis
     

27 Sep, 2012

1 commit


31 Jul, 2012

1 commit


01 Nov, 2011

1 commit

  • Standardize the style for compiler based printf format verification.
    Standardized the location of __printf too.

    Done via script and a little typing.

    $ grep -rPl --include=*.[ch] -w "__attribute__" * | \
    grep -vP "^(tools|scripts|include/linux/compiler-gcc.h)" | \
    xargs perl -n -i -e 'local $/; while (<>) { s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.+)\s*,\s*(.+)\s*\)\s*\)\s*\)/__printf($1, $2)/g ; print; }'

    [akpm@linux-foundation.org: revert arch bits]
    Signed-off-by: Joe Perches
    Cc: "Kirill A. Shutemov"
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

27 Jul, 2011

1 commit

  • This allows us to move duplicated code in
    (atomic_inc_not_zero() for now) to

    Signed-off-by: Arun Sharma
    Reviewed-by: Eric Dumazet
    Cc: Ingo Molnar
    Cc: David Miller
    Cc: Eric Dumazet
    Acked-by: Mike Frysinger
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Arun Sharma
     

23 Oct, 2010

2 commits

  • Fix the following sparse warnings:

    kdb_main.c:328:5: warning: symbol 'kdbgetu64arg' was not declared. Should it be static?
    kgdboc.c:246:12: warning: symbol 'kgdboc_early_init' was not declared. Should it be static?
    kgdb.c:652:26: warning: incorrect type in argument 1 (different address spaces)
    kgdb.c:652:26: expected void const *ptr
    kgdb.c:652:26: got struct perf_event *[noderef] *pev

    The one in kgdb.c required the (void * __force) because of the return
    code from register_wide_hw_breakpoint looking like:

    return (void __percpu __force *)ERR_PTR(err);

    Signed-off-by: Jason Wessel

    Jason Wessel
     
  • In order to allow kernel modules to dynamically add a command to the
    kdb shell the kdb_register, kdb_register_repeat, kdb_unregister, and
    kdb_printf need to be exported as GPL symbols.

    Any kernel module that adds a dynamic kdb shell function should only
    need to include linux/kdb.h.

    Signed-off-by: Jason Wessel

    Jason Wessel
     

05 Aug, 2010

1 commit

  • The kernel console interface stores the number of lines it is
    configured to use. The kdb debugger can greatly benefit by knowing how
    many lines there are on the console for the pager functionality
    without having the end user compile in the setting or have to
    repeatedly change it at run time.

    Signed-off-by: Jason Wessel
    Signed-off-by: Jesse Barnes
    CC: David Airlie
    CC: Andrew Morton

    Jason Wessel
     

21 May, 2010

3 commits

  • Certain calls from the kdb shell will call out to printk(), and any of
    these calls should get vectored back to the kdb_printf() so that the
    kdb pager and processing can be used, as well as to properly channel
    I/O to the polled I/O devices.

    CC: Randy Dunlap
    Signed-off-by: Jason Wessel
    Acked-by: Andrew Morton

    Jason Wessel
     
  • The design of the kdb shell requires that every device that can
    provide input to kdb have a polling routine that exits immediately if
    there is no character available. This is required in order to get the
    page scrolling mechanism working.

    Changing the kernel debugger I/O API to require all polling character
    routines to exit immediately if there is no data allows the kernel
    debugger to process multiple input channels.

    NO_POLL_CHAR will be the return code to the polling routine when ever
    there is no character available.

    CC: linux-serial@vger.kernel.org
    Signed-off-by: Jason Wessel

    Jason Wessel
     
  • This patch contains only the kdb core. Because the change set was
    large, it was split. The next patch in the series includes the
    instrumentation into the core kernel which are mainly helper functions
    for kdb.

    This work is directly derived from kdb v4.4 found at:

    ftp://oss.sgi.com/projects/kdb/download/v4.4/

    The kdb internals have been re-organized to make them mostly platform
    independent and to connect everything to the debug core which is used by
    gdbstub (which has long been known as kgdb).

    The original version of kdb was 58,000 lines worth of changes to
    support x86. From that implementation only the kdb shell, and basic
    commands for memory access, runcontrol, lsmod, and dmesg where carried
    forward.

    This is a generic implementation which aims to cover all the current
    architectures using the kgdb core: ppc, arm, x86, mips, sparc, sh and
    blackfin. More archictectures can be added by implementing the
    architecture specific kgdb functions.

    [mort@sgi.com: Compile fix with hugepages enabled]
    [mort@sgi.com: Clean breakpoint code renaming kdba_ -> kdb_]
    [mort@sgi.com: fix new line after printing registers]
    [mort@sgi.com: Remove the concept of global vs. local breakpoints]
    [mort@sgi.com: Rework kdb_si_swapinfo to use more generic name]
    [mort@sgi.com: fix the information dump macros, remove 'arch' from the names]
    [sfr@canb.auug.org.au: include fixup to include linux/slab.h]

    CC: linux-arch@vger.kernel.org
    Signed-off-by: Jason Wessel
    Signed-off-by: Martin Hicks

    Jason Wessel