linux-kernel - Re: [PATCH AUTOSEL 5.4 31/80] kgdb: Make "kgdbcon" work properly with "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] [day] [month] [year] [list]

Date:   Tue, 27 Oct 2020 12:01:34 +0000
From:   Daniel Thompson <daniel.thompson@...aro.org>
To:     Sasha Levin <sashal@...nel.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Douglas Anderson <dianders@...omium.org>,
        kgdb-bugreport@...ts.sourceforge.net
Subject: Re: [PATCH AUTOSEL 5.4 31/80] kgdb: Make "kgdbcon" work properly
 with "kgdb_earlycon"

On Mon, Oct 26, 2020 at 07:54:27PM -0400, Sasha Levin wrote:
> From: Douglas Anderson <dianders@...omium.org>
> 
> [ Upstream commit b18b099e04f450cdc77bec72acefcde7042bd1f3 ]
> 
> On my system the kernel processes the "kgdb_earlycon" parameter before
> the "kgdbcon" parameter.  When we setup "kgdb_earlycon" we'll end up
> in kgdb_register_callbacks() and "kgdb_use_con" won't have been set
> yet so we'll never get around to starting "kgdbcon".  Let's remedy
> this by detecting that the IO module was already registered when
> setting "kgdb_use_con" and registering the console then.
> 
> As part of this, to avoid pre-declaring things, move the handling of
> the "kgdbcon" further down in the file.
> 
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> Link: https://lore.kernel.org/r/20200630151422.1.I4aa062751ff5e281f5116655c976dff545c09a46@changeid
> Signed-off-by: Daniel Thompson <daniel.thompson@...aro.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>

kgdb[oc]_earlycon was a new feature introduced in v5.8 so, based on the
summary above, this fix does not obviously apply to older kernels.

However after looking closely...

I think the issue described above would also occur if kgdbdbgp (an
incomprehensible sequence consonants that translates to "present
debugger via USB EHCI debug") were used in conjunction with kgdbcon
meaning backporting does make sense.


Daniel.


> ---
>  kernel/debug/debug_core.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 2222f3225e53d..097ab02989f92 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -96,14 +96,6 @@ int dbg_switch_cpu;
>  /* Use kdb or gdbserver mode */
>  int dbg_kdb_mode = 1;
>  
> -static int __init opt_kgdb_con(char *str)
> -{
> -	kgdb_use_con = 1;
> -	return 0;
> -}
> -
> -early_param("kgdbcon", opt_kgdb_con);
> -
>  module_param(kgdb_use_con, int, 0644);
>  module_param(kgdbreboot, int, 0644);
>  
> @@ -876,6 +868,20 @@ static struct console kgdbcons = {
>  	.index		= -1,
>  };
>  
> +static int __init opt_kgdb_con(char *str)
> +{
> +	kgdb_use_con = 1;
> +
> +	if (kgdb_io_module_registered && !kgdb_con_registered) {
> +		register_console(&kgdbcons);
> +		kgdb_con_registered = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +early_param("kgdbcon", opt_kgdb_con);
> +
>  #ifdef CONFIG_MAGIC_SYSRQ
>  static void sysrq_handle_dbg(int key)
>  {
> -- 
> 2.25.1
>