linux-kernel - Re: [PATCH] kgdboc: disable the console lock when in kgdb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 26 Jul 2019 11:16:30 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Douglas Anderson <dianders@...omium.org>
Cc:     Jason Wessel <jason.wessel@...driver.com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Sam Ravnborg <sam@...nborg.org>, Jiri Slaby <jslaby@...e.com>,
        kgdb-bugreport@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
        linux-serial@...r.kernel.org
Subject: Re: [PATCH] kgdboc: disable the console lock when in kgdb

On Thu, Jul 25, 2019 at 11:35:51AM -0700, Douglas Anderson wrote:
> After commit ddde3c18b700 ("vt: More locking checks") kdb / kgdb has
> become useless because my console is filled with spews of:
> 
> WARNING: CPU: 0 PID: 0 at .../drivers/tty/vt/vt.c:3846 con_is_visible+0x50/0x74
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.3.0-rc1+ #48
> Hardware name: Rockchip (Device Tree)
> Backtrace:
> [<c020ce9c>] (dump_backtrace) from [<c020d188>] (show_stack+0x20/0x24)
> [<c020d168>] (show_stack) from [<c0a8fc14>] (dump_stack+0xb0/0xd0)
> [<c0a8fb64>] (dump_stack) from [<c0232c58>] (__warn+0xec/0x11c)
> [<c0232b6c>] (__warn) from [<c0232dc4>] (warn_slowpath_null+0x4c/0x58)
> [<c0232d78>] (warn_slowpath_null) from [<c06338a0>] (con_is_visible+0x50/0x74)
> [<c0633850>] (con_is_visible) from [<c0634078>] (con_scroll+0x108/0x1ac)
> [<c0633f70>] (con_scroll) from [<c0634160>] (lf+0x44/0x88)
> [<c063411c>] (lf) from [<c06363ec>] (vt_console_print+0x1a4/0x2bc)
> [<c0636248>] (vt_console_print) from [<c02f628c>] (vkdb_printf+0x420/0x8a4)
> [<c02f5e6c>] (vkdb_printf) from [<c02f6754>] (kdb_printf+0x44/0x60)
> [<c02f6714>] (kdb_printf) from [<c02fa6f4>] (kdb_main_loop+0xf4/0x6e0)
> [<c02fa600>] (kdb_main_loop) from [<c02fd5f0>] (kdb_stub+0x268/0x398)
> [<c02fd388>] (kdb_stub) from [<c02f3ba0>] (kgdb_cpu_enter+0x1f8/0x674)
> [<c02f39a8>] (kgdb_cpu_enter) from [<c02f4330>] (kgdb_handle_exception+0x1c4/0x1fc)
> [<c02f416c>] (kgdb_handle_exception) from [<c0210fe0>] (kgdb_compiled_brk_fn+0x30/0x3c)
> [<c0210fb0>] (kgdb_compiled_brk_fn) from [<c020d7ac>] (do_undefinstr+0x180/0x1a0)
> [<c020d62c>] (do_undefinstr) from [<c0201b44>] (__und_svc_finish+0x0/0x3c)
> ...
> [<c02f3224>] (kgdb_breakpoint) from [<c02f3310>] (sysrq_handle_dbg+0x58/0x6c)
> [<c02f32b8>] (sysrq_handle_dbg) from [<c062abf0>] (__handle_sysrq+0xac/0x154)
> 
> Let's disable this warning when we're in kgdb to avoid the spew.  The
> whole system is stopped when we're in kgdb so we can't exactly wait
> for someone else to drop the lock.  Presumably the best we can do is
> to disable the warning and hope for the best.
> 
> Fixes: ddde3c18b700 ("vt: More locking checks")
> Cc: Daniel Vetter <daniel.vetter@...el.com>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> 
>  drivers/tty/serial/kgdboc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index bfe5e9e034ec..c7d51b51898f 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -277,10 +277,14 @@ static void kgdboc_pre_exp_handler(void)
>  	/* Increment the module count when the debugger is active */
>  	if (!kgdb_connected)
>  		try_module_get(THIS_MODULE);
> +
> +	atomic_inc(&ignore_console_lock_warning);
>  }
>  
>  static void kgdboc_post_exp_handler(void)
>  {
> +	atomic_dec(&ignore_console_lock_warning);
> +
>  	/* decrement the module count when the debugger detaches */
>  	if (!kgdb_connected)
>  		module_put(THIS_MODULE);
> -- 
> 2.22.0.709.g102302147b-goog

I have the following patch in my tree to go to Linus that I think might
fix this issue for you.  Can you test it instead?

thanks,

greg k-h

-----------------


>From 61d51456f35760a09e8aa1e6ddd247f1547015d3 Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter@...ll.ch>
Date: Thu, 18 Jul 2019 10:09:03 +0200
Subject: [PATCH] vt: Grab console_lock around con_is_bound in show_bind

Not really harmful not to, but also not harm in grabbing the lock. And
this shuts up a new WARNING I introduced in commit ddde3c18b700 ("vt:
More locking checks").

Reported-by: Jens Remus <jremus@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org
Cc: dri-devel@...ts.freedesktop.org
Cc: linux-fbdev@...r.kernel.org
Cc: linux-s390@...r.kernel.org
Cc: Nicolas Pitre <nicolas.pitre@...aro.org>
Cc: Martin Hostettler <textshell@...uujin.de>
Cc: Adam Borowski <kilobyte@...band.pl>
Cc: Mikulas Patocka <mpatocka@...hat.com>
Cc: Daniel Vetter <daniel.vetter@...ll.ch>
Cc: Sam Ravnborg <sam@...nborg.org>
Fixes: ddde3c18b700 ("vt: More locking checks")
Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
Tested-by: Jens Remus <jremus@...ux.ibm.com>
Acked-by: Sam Ravnborg <sam@...nborg.org>
Link: https://lore.kernel.org/r/20190718080903.22622-1-daniel.vetter@ffwll.ch
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 drivers/tty/vt/vt.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index ec92f36ab5c4..34aa39d1aed9 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3771,7 +3771,11 @@ static ssize_t show_bind(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct con_driver *con = dev_get_drvdata(dev);
-	int bind = con_is_bound(con->con);
+	int bind;
+
+	console_lock();
+	bind = con_is_bound(con->con);
+	console_unlock();
 
 	return snprintf(buf, PAGE_SIZE, "%i\n", bind);
 }
-- 
2.22.0