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]

Date:   Fri, 17 Feb 2017 10:25:34 -0200
From:   "Guilherme G. Piccoli" <gpiccoli@...ux.vnet.ibm.com>
To:     Pan Xinhui <xinhui@...ux.vnet.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>
Cc:     linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        paulus@...ba.org, npiggin@...il.com
Subject: Re: [PATCH] powerpc/xmon: Fix an unexpected xmon onoff state change

On 02/17/2017 07:30 AM, Pan Xinhui wrote:
> 
> 
> 在 2017/2/17 14:05, Michael Ellerman 写道:
>> Pan Xinhui <xinhui@...ux.vnet.ibm.com> writes:
>>> 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);
>>
>> I think the logic would probably clearer if we invert this to become
>> xmon_on.
>>
> yep, make sense.
> 
>>> @@ -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;
>>
>> You've just changed the timing of when xmon gets enabled for the above
>> two cases, from here which is called very early, to xmon_setup() which
>> is called much later in boot.
>>
>> That effectively disables xmon for most of the boot, which we do not
>> want to do.
>>
> Although it is not often that kernel got stucked during boot. Yes, the behavior changed anyway. Will fix that in v3.

Pan/Michael, I'm working my patches on top of Pan's. So, I sent his V2
on my series, as patch #1.

Guess the workflow is better/easier if we can work the patches on the
series exclusively, since each time Pan's patch is changed, I need to
refactor my patches.

Pan, if possible send your V3 to me, I'll refactor my series on top of
it and send again on the list. Or if you or Michael have better
suggestions of workflow, let me know.

Thanks,


Guilherme

> 
>> cheers
>>
>