kdb: push 'bt' command output to console immediately.

[5.4-rt] kdb: push 'bt' command output to console immediately.

The rt patch for 5.4 and 5.2 broke kdb slightly. The kdb 'bt' command now prints a single line then returns to the kdb prompt. There is no stack trace being shown.

The missing stack trace is in fact being generated, but is instead being spooled into an internal buffer.

On kdb return-to-kernel (ie, the 'go' command), the spooled output is displayed.

This behavour occurs because kdb calls a non-kdb function, stack_dump(), to decode and dump the stack. That function uses printk's. In an NMI handler (of which kgdb/kdb is an example), printk spools all output generated, and prints that spooled output once the handler exits.

To fix this, mainline has long had a tap in printk that forces it to divert to kdb_printf whenever kdb invokes some out-of-kdb function. That tap, in 5.4-rt, is for some reason no longer functional. This might have something to do with the printk subsystem partial rewrite now present in the rt patch.

Here is my test sequence. This example assumes one is using a serial console not a KVM.

echo ttyS0 > /sys/module/kgdboc/parameters/kgdboc echo g > /proc/sysrq-trigger
bt
go

Here are the kernels I ran the above test on:

4.19.120-rt52 -- Pass

   5.0.21-rt16   -- Pass (*)
   5.2.21-rt15   -- Fail
   5.4.40-rt24   -- Fail

* - Passed on 2nd boot. First boot locked the system up on the 'echo g'. No oops.

I have attached a small patch that Seems To Work. It taps earlier into printk than the official tap does.

Index: b/kernel/printk/printk.c
===================================================================
@@ -2054,7 +2054,10 @@ asmlinkage __visible int printk(const ch
 	int r;
 
 	va_start(args, fmt);
-	r = vprintk_func(fmt, args);
+	if (kdb_trap_printk)
+		r = vkdb_printf(KDB_MSGSRC_INTERNAL, fmt, args);
+	else
+		r = vprintk_func(fmt, args);
 	va_end(args);
 
 	return r;

...

On LKML a similar patch was recently posted[0]. It would probably be better to follow that (patching vprintk_func and using KDB_MSGSRC_PRINTK).

John Ogness

[0] https://lkml.kernel.org/r/20200520102233.GC3464@linux-b0ei

Thanks. It is weird that both this and my bug report were posted on the same day !

Joe

Should I do here anything?

Sebastian

Hi John,
Probably not.

Since the bug is in mainline, not rt, ideally rt should just wait for the fix you so graciously found for me to enter mainline and propagate down to the various stable trees.

That should be OK since the bug is more of an annoyance rather than something critical, so I feel there is no rush to get an official fix in.

For myself, I have applied the fix to my tree, and I will delete it when and whenever it shows up in the mainline branches I care about.

Joe

...

Hi Sebastian,
Oops, my mistake .. the bug is in rt, not mainline. The status of the long-term rt's w.r.t. this patch is:

       5.6-rt    -- already has fix
       5.4-rt    -- needs fix
       4.19-rt   -- needs fix
       4.14-rt   -- needs fix
       4.9-rt    -- needs fix
       4.4-rt    -- needs fix

For your convenience, I've attached the needed patch.

Regards,
Joe

The way user struct reference counting works changed significantly with,

fda31c50292a ("signal: avoid double atomic counter increments for user accounting")

Now user structs are only freed once the last pending signal is dequeued. Make sigqueue_free_current() follow this new convention to avoid freeing the user struct multiple times and triggering this warning:

refcount_t: underflow; use-after-free.

 WARNING: CPU: 0 PID: 6794 at lib/refcount.c:288 refcount_dec_not_one+0x45/0x50
 Call Trace:
  refcount_dec_and_lock_irqsave+0x16/0x60
  free_uid+0x31/0xa0
  ? schedule_hrtimeout_range_clock+0x104/0x110
  __dequeue_signal+0x17c/0x190
  dequeue_signal+0x5a/0x1b0
  do_sigtimedwait+0x208/0x250
  __x64_sys_rt_sigtimedwait+0x6f/0xd0
  do_syscall_64+0x72/0x200
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Index: b/kernel/signal.c
===================================================================
@@ -494,8 +494,8 @@ static void sigqueue_free_current(struct
 
 	up = q->user;
 	if (rt_prio(current->normal_prio) && !put_task_cache(current, q)) {
-		atomic_dec(&up->sigpending);
-		free_uid(up);
+		if (atomic_dec_and_test(&up->sigpending))
+			free_uid(up);
 	} else
 		  __sigqueue_free(q);
 }