linux-kernel - Re: [PATCH] powerpc/xmon: Fix an unexpected xmon onoff state change

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]

Message-Id: <8466dffc-d75c-d83a-2a4f-d567721db7d5@linux.vnet.ibm.com>
Date:   Thu, 16 Feb 2017 19:51:27 +0800
From:   Pan Xinhui <xinhui@...ux.vnet.ibm.com>
To:     "Guilherme G. Piccoli" <gpiccoli@...ux.vnet.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Pan Xinhui <xinhui.pan@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Cc:     benh@...nel.crashing.org, paulus@...ba.org, npiggin@...il.com
Subject: Re: [PATCH] powerpc/xmon: Fix an unexpected xmon onoff state change



在 2017/2/16 18:57, Guilherme G. Piccoli 写道:
> On 16/02/2017 03:09, Michael Ellerman wrote:
>> Pan Xinhui <xinhui.pan@...ux.vnet.ibm.com> writes:
>>
>>> Once xmon is triggered by sysrq-x, it is enabled always afterwards even
>>> if it is disabled during boot. This will cause a system reset interrut
>>> fail to dump. So keep xmon in its original state after exit.
>>>
>>> Signed-off-by: Pan Xinhui <xinhui.pan@...ux.vnet.ibm.com>
>>> ---
>>>  arch/powerpc/xmon/xmon.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> index 9c0e17c..721212f 100644
>>> --- a/arch/powerpc/xmon/xmon.c
>>> +++ b/arch/powerpc/xmon/xmon.c
>>> @@ -76,6 +76,7 @@ static int xmon_gate;
>>>  #endif /* CONFIG_SMP */
>>>
>>>  static unsigned long in_xmon __read_mostly = 0;
>>> +static int xmon_off = 0;
>>>
>>>  static unsigned long adrs;
>>>  static int size = 1;
>>> @@ -3250,6 +3251,8 @@ static void sysrq_handle_xmon(int key)
>>>  	/* ensure xmon is enabled */
>>>  	xmon_init(1);
>>>  	debugger(get_irq_regs());
>>> +	if (xmon_off)
>>> +		xmon_init(0);
>>>  }
>>
>> I don't think this is right.
>>
>> xmon_off is only true if you boot with xmon=off on the command line.
>>
>> So if you boot with CONFIG_XMON_DEFAULT=n, and nothing on the command
>> line, then enter xmon via sysrq, then exit, xmon will still be enabled.
>>
>
> Agreed, noticed it after some work in V2 of my patch. I'm addressing it
> there, so maybe no harm in keeping this way here..
>
Hi, mpe
	I cooked a new patch. And as Paul mentioned in slack, we need keep xmon on too if xmon=early is set in cmdline.

hi, Guilherme
	feel free to include it in your new patchset. :)

thanks
xinhui

--------patch-----
powerpc/xmon: Fix an unexpected xmon onoff state change

Once xmon is triggered by sysrq-x, it is enabled always afterwards even
if it is disabled during boot. This will cause a system reset interrupt
fail to dump. So keep xmon in its original state after exit.

We have several ways to set xmon on or off.
1) by a build config CONFIG_XMON_DEFAULT.
2) by a boot cmdline with xmon or xmon=early or xmon=on to enable xmon
and xmon=off to disable xmon. This value will override that in step 1.
3) by a debugfs interface. We need someone implement it in the future.
And this value can override those in step 1 and 2.

Signed-off-by: Pan Xinhui <xinhui.pan@...ux.vnet.ibm.com>
---
  arch/powerpc/xmon/xmon.c | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 9c0e17c..f6e5c3d 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -76,6 +76,7 @@ static int xmon_gate;
  #endif /* CONFIG_SMP */
  
  static unsigned long in_xmon __read_mostly = 0;
+static int xmon_off = !IS_ENABLED(CONFIG_XMON_DEFAULT);
  
  static unsigned long adrs;
  static int size = 1;
@@ -3250,6 +3251,8 @@ static void sysrq_handle_xmon(int key)
  	/* ensure xmon is enabled */
  	xmon_init(1);
  	debugger(get_irq_regs());
+	if (xmon_off)
+		xmon_init(0);
  }
  
  static struct sysrq_key_op sysrq_xmon_op = {
@@ -3266,16 +3269,16 @@ static int __init setup_xmon_sysrq(void)
  __initcall(setup_xmon_sysrq);
  #endif /* CONFIG_MAGIC_SYSRQ */
  
-static int __initdata xmon_early, xmon_off;
+static int __initdata xmon_early;
  
  static int __init early_parse_xmon(char *p)
  {
  	if (!p || strncmp(p, "early", 5) == 0) {
  		/* just "xmon" is equivalent to "xmon=early" */
-		xmon_init(1);
  		xmon_early = 1;
+		xmon_off = 0;
  	} else if (strncmp(p, "on", 2) == 0)
-		xmon_init(1);
+		xmon_off = 0;
  	else if (strncmp(p, "off", 3) == 0)
  		xmon_off = 1;
  	else if (strncmp(p, "nobt", 4) == 0)
@@ -3289,10 +3292,9 @@ early_param("xmon", early_parse_xmon);
  
  void __init xmon_setup(void)
  {
-#ifdef CONFIG_XMON_DEFAULT
-	if (!xmon_off)
-		xmon_init(1);
-#endif
+	if (xmon_off)
+		return;
+	xmon_init(1);
  	if (xmon_early)
  		debugger(NULL);
  }
-- 
2.9.3



> Thanks,
>
>
> Guilherme
>
>
>
>> cheers
>>
>