[PATCH] arm64: fix for bad_mode() handler to always result in panic

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Hari Vyas]

bad_mode() handler is called for invalid or undefined
instruction in el1 level or when irq,fiq,sync or error
situation happen in el1 or el0 level.

As per latest code, above abnormal situation may not result in
panic always due to die() call if user mode is determined at
that moment. That will just result in kill of current process
and panic will be avoided which it must not.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=200637
Signed-off-by: Hari Vyas <hari.vyas@broadcom.com>
---
 arch/arm64/kernel/traps.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index d399d45..716ee73 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -621,7 +621,6 @@ asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr)
 		handler[reason], smp_processor_id(), esr,
 		esr_get_class_string(esr));
 
-	die("Oops - bad mode", regs, 0);
 	local_daif_mask();
 	panic("bad mode");
 }
-- 
1.9.1

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Greg KH]

On Tue, Aug 07, 2018 at 04:33:48PM +0530, Hari Vyas wrote:
> bad_mode() handler is called for invalid or undefined
> instruction in el1 level or when irq,fiq,sync or error
> situation happen in el1 or el0 level.
> 
> As per latest code, above abnormal situation may not result in
> panic always due to die() call if user mode is determined at
> that moment. That will just result in kill of current process
> and panic will be avoided which it must not.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200637
> Signed-off-by: Hari Vyas <hari.vyas@broadcom.com>
> ---
>  arch/arm64/kernel/traps.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index d399d45..716ee73 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -621,7 +621,6 @@ asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr)
>  		handler[reason], smp_processor_id(), esr,
>  		esr_get_class_string(esr));
>  
> -	die("Oops - bad mode", regs, 0);
>  	local_daif_mask();
>  	panic("bad mode");
>  }
> -- 
> 1.9.1

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Hari Vyas]

On Tue, Aug 7, 2018 at 5:57 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Aug 07, 2018 at 04:33:48PM +0530, Hari Vyas wrote:
>> bad_mode() handler is called for invalid or undefined
>> instruction in el1 level or when irq,fiq,sync or error
>> situation happen in el1 or el0 level.
>>
>> As per latest code, above abnormal situation may not result in
>> panic always due to die() call if user mode is determined at
>> that moment. That will just result in kill of current process
>> and panic will be avoided which it must not.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200637
>> Signed-off-by: Hari Vyas <hari.vyas@broadcom.com>
>> ---
>>  arch/arm64/kernel/traps.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index d399d45..716ee73 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>> @@ -621,7 +621,6 @@ asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr)
>>               handler[reason], smp_processor_id(), esr,
>>               esr_get_class_string(esr));
>>
>> -     die("Oops - bad mode", regs, 0);
>>       local_daif_mask();
>>       panic("bad mode");
>>  }
>> --
>> 1.9.1
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
>
> </formletter>

Thanks. Probably my mistake to include stable kernel list for regular
minor patch.
In any case, will take care about it along with arm maintainers and
developers review comments which I am awaiting.

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Florian Fainelli]

On 08/08/2018 11:18 AM, Hari Vyas wrote:
> On Tue, Aug 7, 2018 at 5:57 PM, Greg KH <greg@kroah.com> wrote:
>> On Tue, Aug 07, 2018 at 04:33:48PM +0530, Hari Vyas wrote:
>>> bad_mode() handler is called for invalid or undefined
>>> instruction in el1 level or when irq,fiq,sync or error
>>> situation happen in el1 or el0 level.
>>>
>>> As per latest code, above abnormal situation may not result in
>>> panic always due to die() call if user mode is determined at
>>> that moment. That will just result in kill of current process
>>> and panic will be avoided which it must not.
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200637
>>> Signed-off-by: Hari Vyas <hari.vyas@broadcom.com>
>>> ---
>>>  arch/arm64/kernel/traps.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>>> index d399d45..716ee73 100644
>>> --- a/arch/arm64/kernel/traps.c
>>> +++ b/arch/arm64/kernel/traps.c
>>> @@ -621,7 +621,6 @@ asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr)
>>>               handler[reason], smp_processor_id(), esr,
>>>               esr_get_class_string(esr));
>>>
>>> -     die("Oops - bad mode", regs, 0);
>>>       local_daif_mask();
>>>       panic("bad mode");
>>>  }
>>> --
>>> 1.9.1
>>
>> <formletter>
>>
>> This is not the correct way to submit patches for inclusion in the
>> stable kernel tree.  Please read:
>>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>> for how to do this properly.
>>
>> </formletter>
> 
> Thanks. Probably my mistake to include stable kernel list for regular
> minor patch.
> In any case, will take care about it along with arm maintainers and
> developers review comments which I am awaiting.

Also, if this is a real fix, then providing an appropriate Fixes: tag
would help understand when this broke, and how far back this needs to be
backported to. This of course, assumes that your patch is the right
course of action.
-- 
Florian

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Hari Vyas]

On Wed, Aug 8, 2018 at 11:53 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 08/08/2018 11:18 AM, Hari Vyas wrote:
>> On Tue, Aug 7, 2018 at 5:57 PM, Greg KH <greg@kroah.com> wrote:
>>> On Tue, Aug 07, 2018 at 04:33:48PM +0530, Hari Vyas wrote:
>>>> bad_mode() handler is called for invalid or undefined
>>>> instruction in el1 level or when irq,fiq,sync or error
>>>> situation happen in el1 or el0 level.
>>>>
>>>> As per latest code, above abnormal situation may not result in
>>>> panic always due to die() call if user mode is determined at
>>>> that moment. That will just result in kill of current process
>>>> and panic will be avoided which it must not.
>>>>
>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200637
>>>> Signed-off-by: Hari Vyas <hari.vyas@broadcom.com>
>>>> ---
>>>>  arch/arm64/kernel/traps.c | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>>>> index d399d45..716ee73 100644
>>>> --- a/arch/arm64/kernel/traps.c
>>>> +++ b/arch/arm64/kernel/traps.c
>>>> @@ -621,7 +621,6 @@ asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr)
>>>>               handler[reason], smp_processor_id(), esr,
>>>>               esr_get_class_string(esr));
>>>>
>>>> -     die("Oops - bad mode", regs, 0);
>>>>       local_daif_mask();
>>>>       panic("bad mode");
>>>>  }
>>>> --
>>>> 1.9.1
>>>
>>> <formletter>
>>>
>>> This is not the correct way to submit patches for inclusion in the
>>> stable kernel tree.  Please read:
>>>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>> for how to do this properly.
>>>
>>> </formletter>
>>
>> Thanks. Probably my mistake to include stable kernel list for regular
>> minor patch.
>> In any case, will take care about it along with arm maintainers and
>> developers review comments which I am awaiting.
>
> Also, if this is a real fix, then providing an appropriate Fixes: tag
> would help understand when this broke, and how far back this needs to be
> backported to. This of course, assumes that your patch is the right
> course of action.
> --
As far as I know, bad mode was earlier(linux 3.14 onwards) also not
resulting in panic always.
Trap-exception framework is recently changed some time back but this
concern remains same.
I am also awaiting a response from ARM maintainers before proceeding.
Just to be more clear, this
concern was pointed out in one of my previous-and-some-what-relative
patch about console-verbose
level restoration issue.
> Florian

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Hari Vyas]

On Fri, Aug 10, 2018 at 10:16 PM, Hari Vyas <hari.vyas@broadcom.com> wrote:
> On Wed, Aug 8, 2018 at 11:53 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 08/08/2018 11:18 AM, Hari Vyas wrote:
>>> On Tue, Aug 7, 2018 at 5:57 PM, Greg KH <greg@kroah.com> wrote:
>>>> On Tue, Aug 07, 2018 at 04:33:48PM +0530, Hari Vyas wrote:
>>>>> bad_mode() handler is called for invalid or undefined
>>>>> instruction in el1 level or when irq,fiq,sync or error
>>>>> situation happen in el1 or el0 level.
>>>>>
>>>>> As per latest code, above abnormal situation may not result in
>>>>> panic always due to die() call if user mode is determined at
>>>>> that moment. That will just result in kill of current process
>>>>> and panic will be avoided which it must not.
>>>>>
>>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200637
>>>>> Signed-off-by: Hari Vyas <hari.vyas@broadcom.com>
>>>>> ---
>>>>>  arch/arm64/kernel/traps.c | 1 -
>>>>>  1 file changed, 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>>>>> index d399d45..716ee73 100644
>>>>> --- a/arch/arm64/kernel/traps.c
>>>>> +++ b/arch/arm64/kernel/traps.c
>>>>> @@ -621,7 +621,6 @@ asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr)
>>>>>               handler[reason], smp_processor_id(), esr,
>>>>>               esr_get_class_string(esr));
>>>>>
>>>>> -     die("Oops - bad mode", regs, 0);
>>>>>       local_daif_mask();
>>>>>       panic("bad mode");
>>>>>  }
>>>>> --
>>>>> 1.9.1
>>>>
>>>> <formletter>
>>>>
>>>> This is not the correct way to submit patches for inclusion in the
>>>> stable kernel tree.  Please read:
>>>>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>> for how to do this properly.
>>>>
>>>> </formletter>
>>>
>>> Thanks. Probably my mistake to include stable kernel list for regular
>>> minor patch.
>>> In any case, will take care about it along with arm maintainers and
>>> developers review comments which I am awaiting.
>>
>> Also, if this is a real fix, then providing an appropriate Fixes: tag
>> would help understand when this broke, and how far back this needs to be
>> backported to. This of course, assumes that your patch is the right
>> course of action.
>> --
> As far as I know, bad mode was earlier(linux 3.14 onwards) also not
> resulting in panic always.
> Trap-exception framework is recently changed some time back but this
> concern remains same.
> I am also awaiting a response from ARM maintainers before proceeding.
> Just to be more clear, this
> concern was pointed out in one of my previous-and-some-what-relative
> patch about console-verbose
> level restoration issue.
>> Florian
Gentle reminder for Arm maintainers.
If concern can be taken care from your side in different ways then bug
can be resolved.

Regards,
hari

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Will Deacon]

Hi Hari,

Sorry, this has slipped through the cracks.

On Tue, Aug 21, 2018 at 07:46:04PM +0530, Hari Vyas wrote:
> On Fri, Aug 10, 2018 at 10:16 PM, Hari Vyas <hari.vyas@broadcom.com> wrote:
> > As far as I know, bad mode was earlier(linux 3.14 onwards) also not
> > resulting in panic always.
> > Trap-exception framework is recently changed some time back but this
> > concern remains same.
> > I am also awaiting a response from ARM maintainers before proceeding.
> > Just to be more clear, this
> > concern was pointed out in one of my previous-and-some-what-relative
> > patch about console-verbose
> > level restoration issue.
> >> Florian
> Gentle reminder for Arm maintainers.
> If concern can be taken care from your side in different ways then bug
> can be resolved.

I've recently started trying to untangle much of our fatal trap handling
code so that the decision between user-mode and kernel-mode actions is
explicit in the caller, rather than buried deep in the bowels of some
callchain. The aim is also to remove arm64_notify_die() entirely, which
is a complete maze of crap that ultimately depends on what kgdb does in
some cases!

With that out of the way, die() itself could probably be simplified so that
it (a) avoids the notifier chain [which is useless for us] and (b) is only
intended to be called for kernel contexts, i.e. as a wrapper around a
panic_on_oops check.

But this needn't affect your patch, which seems to be a small step in the
right direction. I'll pick it up as part of my series when I send it out,
if that's alright with you? I don't think any of this is especially urgent.

Will

[PATCH] arm64: fix for bad_mode() handler to always result in panic

[Hari Vyas]

On Tue, Aug 21, 2018 at 11:08 PM, Will Deacon <will.deacon@arm.com> wrote:
> Hi Hari,
>
> Sorry, this has slipped through the cracks.
>
> On Tue, Aug 21, 2018 at 07:46:04PM +0530, Hari Vyas wrote:
>> On Fri, Aug 10, 2018 at 10:16 PM, Hari Vyas <hari.vyas@broadcom.com> wrote:
>> > As far as I know, bad mode was earlier(linux 3.14 onwards) also not
>> > resulting in panic always.
>> > Trap-exception framework is recently changed some time back but this
>> > concern remains same.
>> > I am also awaiting a response from ARM maintainers before proceeding.
>> > Just to be more clear, this
>> > concern was pointed out in one of my previous-and-some-what-relative
>> > patch about console-verbose
>> > level restoration issue.
>> >> Florian
>> Gentle reminder for Arm maintainers.
>> If concern can be taken care from your side in different ways then bug
>> can be resolved.
>
> I've recently started trying to untangle much of our fatal trap handling
> code so that the decision between user-mode and kernel-mode actions is
> explicit in the caller, rather than buried deep in the bowels of some
> callchain. The aim is also to remove arm64_notify_die() entirely, which
> is a complete maze of crap that ultimately depends on what kgdb does in
> some cases!
>
> With that out of the way, die() itself could probably be simplified so that
> it (a) avoids the notifier chain [which is useless for us] and (b) is only
> intended to be called for kernel contexts, i.e. as a wrapper around a
> panic_on_oops check.
>
> But this needn't affect your patch, which seems to be a small step in the
> right direction. I'll pick it up as part of my series when I send it out,
> if that's alright with you? I don't think any of this is especially urgent.
>
> Will
Thanks Will for your reply. Issue should be just addressed. If it is
being taken care from
your end in better way, no issue from our end too. Please intimate
once changes are
available in kernel release so that we can pick it.

Regards,
hari