Skip to content

Commit

Permalink
serial: kgdboc: Fix NMI-safety problems from keyboard reset code
Browse files Browse the repository at this point in the history
commit b2aba15ad6f908d1a620fd97f6af5620c3639742 upstream.

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
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20240424-kgdboc_fix_schedule_work-v2-1-50f5a490aec5@linaro.org
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
daniel-thompson authored and gregkh committed May 25, 2024
1 parent 3fe1726 commit 512b938
Showing 1 changed file with 29 additions and 1 deletion.
30 changes: 29 additions & 1 deletion drivers/tty/serial/kgdboc.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <linux/console.h>
#include <linux/vt_kern.h>
#include <linux/input.h>
#include <linux/irq_work.h>
#include <linux/module.h>

#define MAX_CONFIG_LEN 40
Expand All @@ -35,6 +36,25 @@ static int kgdboc_use_kms; /* 1 if we use kernel mode switching */
static struct tty_driver *kgdb_tty_driver;
static int kgdb_tty_line;

/*
* 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
* system workqueue, which runs in task context.
*/
#ifdef CONFIG_KDB_KEYBOARD
static int kgdboc_reset_connect(struct input_handler *handler,
struct input_dev *dev,
Expand Down Expand Up @@ -86,10 +106,17 @@ static void kgdboc_restore_input_helper(struct work_struct *dummy)

static DECLARE_WORK(kgdboc_restore_input_work, kgdboc_restore_input_helper);

static void kgdboc_queue_restore_input_helper(struct irq_work *unused)
{
schedule_work(&kgdboc_restore_input_work);
}

static DEFINE_IRQ_WORK(kgdboc_restore_input_irq_work, kgdboc_queue_restore_input_helper);

static void kgdboc_restore_input(void)
{
if (likely(system_state == SYSTEM_RUNNING))
schedule_work(&kgdboc_restore_input_work);
irq_work_queue(&kgdboc_restore_input_irq_work);
}

static int kgdboc_register_kbd(char **cptr)
Expand Down Expand Up @@ -120,6 +147,7 @@ static void kgdboc_unregister_kbd(void)
i--;
}
}
irq_work_sync(&kgdboc_restore_input_irq_work);
flush_work(&kgdboc_restore_input_work);
}
#else /* ! CONFIG_KDB_KEYBOARD */
Expand Down

0 comments on commit 512b938

Please sign in to comment.