History

Merge tag 'kgdb-5.8-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux

Pull kgdb fixes from Daniel Thompson: "The main change here is a fix for a number of unsafe interactions between kdb and the console system. The fixes are specific to kdb (pure kgdb debugging does not use the console system at all). On systems with an NMI then kdb, if it is enabled, must get messages to the user despite potentially running from some "difficult" calling contexts. These fixes avoid using the console system where we have been provided an alternative (safer) way to interact with the user and, if using the console system in unavoidable, use oops_in_progress for deadlock avoidance. These fixes also ensure kdb honours the console enable flag. Also included is a fix that wraps kgdb trap handling in an RCU read lock to avoids triggering diagnostic warnings. This is a wide lock scope but this is OK because kgdb is a stop-the-world debugger. When we stop the world we put all the CPUs into holding pens and this inhibits RCU update anyway" * tag 'kgdb-5.8-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/danielt/linux: kgdb: Avoid suspicious RCU usage warning kdb: Switch to use safer dbg_io_ops over console APIs kdb: Make kdb_printf() console handling more robust kdb: Check status of console prior to invoking handlers kdb: Re-factor kdb_printf() message write code

Linus Torvalds committed 6 years ago

Commit 6116dea2 parents 21d2f68 440ab9e6 files · +68 −50Browse files

Unified · Split

6 files
  1. drivers/tty/serial/kgdb_nmi.c +1 −1
  2. drivers/tty/serial/kgdboc.c +16 −16
  3. drivers/usb/early/ehci-dbgp.c +2 −1
  4. include/linux/kgdb.h +2 −3
  5. kernel/debug/debug_core.c +4 −0
  6. kernel/debug/kdb/kdb_io.c +43 −29
drivers/tty/serial/kgdb_nmi.c+1−1
@@ -50,7 +50,7 @@ static int kgdb_nmi_console_setup(struct console *co, char *options) · 49 unmodified lines
5050 * I/O utilities that messages sent to the console will automatically
5151 * be displayed on the dbg_io.
5252 */
53− dbg_io_ops->is_console = true;
53+ dbg_io_ops->cons = co;
5454
5555 return 0;
5656 }
drivers/tty/serial/kgdboc.c+16−16
@@ -45,7 +45,6 @@ static struct platform_device *kgdboc_pdev; · 44 unmodified lines
4545
4646 #if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
4747 static struct kgdb_io kgdboc_earlycon_io_ops;
48−static struct console *earlycon;
4948 static int (*earlycon_orig_exit)(struct console *con);
5049 #endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
5150
@@ -145,7 +144,7 @@ static void kgdboc_unregister_kbd(void) · 93 unmodified lines
145144 #if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
146145 static void cleanup_earlycon(void)
147146 {
148− if (earlycon)
147+ if (kgdboc_earlycon_io_ops.cons)
149148 kgdb_unregister_io_module(&kgdboc_earlycon_io_ops);
150149 }
151150 #else /* !IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
@@ -178,7 +177,7 @@ static int configure_kgdboc(void) · 26 unmodified lines
178177 goto noconfig;
179178 }
180179
181− kgdboc_io_ops.is_console = 0;
180+ kgdboc_io_ops.cons = NULL;
182181 kgdb_tty_driver = NULL;
183182
184183 kgdboc_use_kms = 0;
@@ -198,7 +197,7 @@ static int configure_kgdboc(void) · 13 unmodified lines
198197 int idx;
199198 if (cons->device && cons->device(cons, &idx) == p &&
200199 idx == tty_line) {
201− kgdboc_io_ops.is_console = 1;
200+ kgdboc_io_ops.cons = cons;
202201 break;
203202 }
204203 }
@@ -433,7 +432,8 @@ static int kgdboc_earlycon_get_char(void) · 228 unmodified lines
433432 {
434433 char c;
435434
436− if (!earlycon->read(earlycon, &c, 1))
435+ if (!kgdboc_earlycon_io_ops.cons->read(kgdboc_earlycon_io_ops.cons,
436+ &c, 1))
437437 return NO_POLL_CHAR;
438438
439439 return c;
@@ -441,7 +441,8 @@ static int kgdboc_earlycon_get_char(void) · 1 unmodified line
441441
442442 static void kgdboc_earlycon_put_char(u8 chr)
443443 {
444− earlycon->write(earlycon, &chr, 1);
444+ kgdboc_earlycon_io_ops.cons->write(kgdboc_earlycon_io_ops.cons, &chr,
445+ 1);
445446 }
446447
447448 static void kgdboc_earlycon_pre_exp_handler(void)
@@ -461,7 +462,7 @@ static void kgdboc_earlycon_pre_exp_handler(void) · 13 unmodified lines
461462 * boot if we detect this case.
462463 */
463464 for_each_console(con)
464− if (con == earlycon)
465+ if (con == kgdboc_earlycon_io_ops.cons)
465466 return;
466467
467468 already_warned = true;
@@ -484,25 +485,25 @@ static int kgdboc_earlycon_deferred_exit(struct console *con) · 16 unmodified lines
484485
485486 static void kgdboc_earlycon_deinit(void)
486487 {
487− if (!earlycon)
488+ if (!kgdboc_earlycon_io_ops.cons)
488489 return;
489490
490− if (earlycon->exit == kgdboc_earlycon_deferred_exit)
491+ if (kgdboc_earlycon_io_ops.cons->exit == kgdboc_earlycon_deferred_exit)
491492 /*
492493 * kgdboc_earlycon is exiting but original boot console exit
493494 * was never called (AKA kgdboc_earlycon_deferred_exit()
494495 * didn't ever run). Undo our trap.
495496 */
496− earlycon->exit = earlycon_orig_exit;
497− else if (earlycon->exit)
497+ kgdboc_earlycon_io_ops.cons->exit = earlycon_orig_exit;
498+ else if (kgdboc_earlycon_io_ops.cons->exit)
498499 /*
499500 * We skipped calling the exit() routine so we could try to
500501 * keep using the boot console even after it went away. We're
501502 * finally done so call the function now.
502503 */
503− earlycon->exit(earlycon);
504+ kgdboc_earlycon_io_ops.cons->exit(kgdboc_earlycon_io_ops.cons);
504505
505− earlycon = NULL;
506+ kgdboc_earlycon_io_ops.cons = NULL;
506507 }
507508
508509 static struct kgdb_io kgdboc_earlycon_io_ops = {
@@ -511,7 +512,6 @@ static struct kgdb_io kgdboc_earlycon_io_ops = { · 2 unmodified lines
511512 .write_char = kgdboc_earlycon_put_char,
512513 .pre_exception = kgdboc_earlycon_pre_exp_handler,
513514 .deinit = kgdboc_earlycon_deinit,
514− .is_console = true,
515515 };
516516
517517 #define MAX_CONSOLE_NAME_LEN (sizeof((struct console *) 0)->name)
@@ -557,10 +557,10 @@ static int __init kgdboc_earlycon_init(char *opt) · 39 unmodified lines
557557 goto unlock;
558558 }
559559
560− earlycon = con;
560+ kgdboc_earlycon_io_ops.cons = con;
561561 pr_info("Going to register kgdb with earlycon '%s'\n", con->name);
562562 if (kgdb_register_io_module(&kgdboc_earlycon_io_ops) != 0) {
563− earlycon = NULL;
563+ kgdboc_earlycon_io_ops.cons = NULL;
564564 pr_info("Failed to register kgdb with earlycon\n");
565565 } else {
566566 /* Trap exit so we can keep earlycon longer if needed. */
drivers/usb/early/ehci-dbgp.c+2−1
@@ -1058,7 +1058,8 @@ static int __init kgdbdbgp_parse_config(char *str) · 1057 unmodified lines
10581058 kgdbdbgp_wait_time = simple_strtoul(ptr, &ptr, 10);
10591059 }
10601060 kgdb_register_io_module(&kgdbdbgp_io_ops);
1061− kgdbdbgp_io_ops.is_console = early_dbgp_console.index != -1;
1061+ if (early_dbgp_console.index != -1)
1062+ kgdbdbgp_io_ops.cons = &early_dbgp_console;
10621063
10631064 return 0;
10641065 }
include/linux/kgdb.h+2−3
@@ -276,8 +276,7 @@ struct kgdb_arch { · 275 unmodified lines
276276 * the I/O driver.
277277 * @post_exception: Pointer to a function that will do any cleanup work
278278 * for the I/O driver.
279− * @is_console: 1 if the end device is a console 0 if the I/O device is
280− * not a console
279+ * @cons: valid if the I/O device is a console; else NULL.
281280 */
282281 struct kgdb_io {
283282 const char *name;
@@ -288,7 +287,7 @@ struct kgdb_io { · 4 unmodified lines
288287 void (*deinit) (void);
289288 void (*pre_exception) (void);
290289 void (*post_exception) (void);
291− int is_console;
290+ struct console *cons;
292291 };
293292
294293 extern const struct kgdb_arch arch_kgdb_ops;
kernel/debug/debug_core.c+4−0
@@ -587,6 +587,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, · 586 unmodified lines
587587 arch_kgdb_ops.disable_hw_break(regs);
588588
589589 acquirelock:
590+ rcu_read_lock();
590591 /*
591592 * Interrupts will be restored by the 'trap return' code, except when
592593 * single stepping.
@@ -646,6 +647,7 @@ return_normal: · 53 unmodified lines
646647 atomic_dec(&slaves_in_kgdb);
647648 dbg_touch_watchdogs();
648649 local_irq_restore(flags);
650+ rcu_read_unlock();
649651 return 0;
650652 }
651653 cpu_relax();
@@ -664,6 +666,7 @@ return_normal: · 12 unmodified lines
664666 raw_spin_unlock(&dbg_master_lock);
665667 dbg_touch_watchdogs();
666668 local_irq_restore(flags);
669+ rcu_read_unlock();
667670
668671 goto acquirelock;
669672 }
@@ -787,6 +790,7 @@ kgdb_restore: · 117 unmodified lines
787790 raw_spin_unlock(&dbg_master_lock);
788791 dbg_touch_watchdogs();
789792 local_irq_restore(flags);
793+ rcu_read_unlock();
790794
791795 return kgdb_info[cpu].ret_state;
792796 }
kernel/debug/kdb/kdb_io.c+43−29
@@ -542,6 +542,44 @@ static int kdb_search_string(char *searched, char *searchfor) · 541 unmodified lines
542542 return 0;
543543 }
544544
545+static void kdb_msg_write(const char *msg, int msg_len)
546+{
547+ struct console *c;
548+
549+ if (msg_len == 0)
550+ return;
551+
552+ if (dbg_io_ops) {
553+ const char *cp = msg;
554+ int len = msg_len;
555+
556+ while (len--) {
557+ dbg_io_ops->write_char(*cp);
558+ cp++;
559+ }
560+ }
561+
562+ for_each_console(c) {
563+ if (!(c->flags & CON_ENABLED))
564+ continue;
565+ if (c == dbg_io_ops->cons)
566+ continue;
567+ /*
568+ * Set oops_in_progress to encourage the console drivers to
569+ * disregard their internal spin locks: in the current calling
570+ * context the risk of deadlock is a bigger problem than risks
571+ * due to re-entering the console driver. We operate directly on
572+ * oops_in_progress rather than using bust_spinlocks() because
573+ * the calls bust_spinlocks() makes on exit are not appropriate
574+ * for this calling context.
575+ */
576+ ++oops_in_progress;
577+ c->write(c, msg, msg_len);
578+ --oops_in_progress;
579+ touch_nmi_watchdog();
580+ }
581+}
582+
545583 int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap)
546584 {
547585 int diag;
@@ -553,7 +591,6 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap) · 5 unmodified lines
553591 int this_cpu, old_cpu;
554592 char *cp, *cp2, *cphold = NULL, replaced_byte = ' ';
555593 char *moreprompt = "more> ";
556− struct console *c;
557594 unsigned long uninitialized_var(flags);
558595
559596 /* Serialize kdb_printf if multiple cpus try to write at once.
@@ -687,22 +724,11 @@ kdb_printit: · 127 unmodified lines
687724 */
688725 retlen = strlen(kdb_buffer);
689726 cp = (char *) printk_skip_headers(kdb_buffer);
690− if (!dbg_kdb_mode && kgdb_connected) {
727+ if (!dbg_kdb_mode && kgdb_connected)
691728 gdbstub_msg_write(cp, retlen - (cp - kdb_buffer));
692− } else {
693− if (dbg_io_ops && !dbg_io_ops->is_console) {
694− len = retlen - (cp - kdb_buffer);
695− cp2 = cp;
696− while (len--) {
697− dbg_io_ops->write_char(*cp2);
698− cp2++;
699− }
700− }
701− for_each_console(c) {
702− c->write(c, cp, retlen - (cp - kdb_buffer));
703− touch_nmi_watchdog();
704− }
705− }
729+ else
730+ kdb_msg_write(cp, retlen - (cp - kdb_buffer));
731+
706732 if (logging) {
707733 saved_loglevel = console_loglevel;
708734 console_loglevel = CONSOLE_LOGLEVEL_SILENT;
@@ -751,19 +777,7 @@ kdb_printit: · 42 unmodified lines
751777 moreprompt = "more> ";
752778
753779 kdb_input_flush();
754−
755− if (dbg_io_ops && !dbg_io_ops->is_console) {
756− len = strlen(moreprompt);
757− cp = moreprompt;
758− while (len--) {
759− dbg_io_ops->write_char(*cp);
760− cp++;
761− }
762− }
763− for_each_console(c) {
764− c->write(c, moreprompt, strlen(moreprompt));
765− touch_nmi_watchdog();
766− }
780+ kdb_msg_write(moreprompt, strlen(moreprompt));
767781
768782 if (logging)
769783 printk("%s", moreprompt);