• Petr Mladek's avatar
    kdb: call vkdb_printf() from vprintk_default() only when wanted · 34aaff40
    Petr Mladek authored
    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.comSigned-off-by: default avatarPetr Mladek <pmladek@suse.com>
    Cc: Daniel Thompson <daniel.thompson@linaro.org>
    Cc: Jason Wessel <jason.wessel@windriver.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    34aaff40
kdb_io.c 20.6 KB