Hi,
On Fri, Apr 19, 2024 at 3:30 AM Daniel Thompson daniel.thompson@linaro.org wrote:
Currently, when kdb is compiled with keyboard support, then we will use schedule_work() to provoke reset of the keyboard status. Unfortunately schedule_work() gets called from the kgdboc post-debug-exception handler. That risks deadlock since schedule_work() is not NMI-safe and, even on platforms where the NMI is not directly used for debugging, the debug trap can have NMI-like behaviour depending on where breakpoints are placed.
Fix this by using the irq work system, which is NMI-safe, to defer the call to schedule_work() to a point when it is safe to call.
Reported-by: Liuye liu.yeC@h3c.com Closes: https://lore.kernel.org/all/20240228025602.3087748-1-liu.yeC@h3c.com/ Cc: stable@vger.kernel.org Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
drivers/tty/serial/kgdboc.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c index 7ce7bb1640054..adcea70fd7507 100644 --- a/drivers/tty/serial/kgdboc.c +++ b/drivers/tty/serial/kgdboc.c @@ -19,6 +19,7 @@ #include <linux/console.h> #include <linux/vt_kern.h> #include <linux/input.h> +#include <linux/irq_work.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/serial_core.h> @@ -48,6 +49,25 @@ static struct kgdb_io kgdboc_earlycon_io_ops; static int (*earlycon_orig_exit)(struct console *con); #endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
+/*
- When we leave the debug trap handler we need to reset the keyboard status
- (since the original keyboard state gets partially clobbered by kdb use of
- the keyboard).
- The path to deliver the reset is somewhat circuitous.
- To deliver the reset we register an input handler, reset the keyboard and
- then deregister the input handler. However, to get this done right, we do
- have to carefully manage the calling context because we can only register
- input handlers from task context.
- In particular we need to trigger the action from the debug trap handler with
- all its NMI and/or NMI-like oddities. To solve this the kgdboc trap exit code
- (the "post_exception" callback) uses irq_work_queue(), which is NMI-safe, to
- schedule a callback from a hardirq context. From there we have to defer the
- work again, this time using schedule_Work(), to get a callback using the
nit: schedule_work() (no capital "W").
- system workqueue, which runs in task context.
Thank you for the comment. It makes the double-jump through IRQ work and then normal work clearer.
Other than the nit in the comment, this looks good to me.
Reviewed-by: Douglas Anderson dianders@chromium.org