linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* SError handling vs. SIGSEGV
@ 2020-03-28  4:31 Florian Fainelli
  2020-03-28 16:43 ` Andrew Lunn
  2020-03-30 11:30 ` James Morse
  0 siblings, 2 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-03-28  4:31 UTC (permalink / raw)
  To: linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland,
	Dave.Martin, james.morse, Doug Berger, bcm-kernel-feedback-list,
	Scott Branden, Ray Jui

Hello,

Up until commit e4ba15debcfd27f60d43da940a58108783bff2a6 ("arm64:
fix for bad_mode() handler to always result in panic") we had been
getting SIGSEGV delivered to applications running on Broadcom STB
platforms which access register holes or registers for which we have
purposely blocked the access via the GISB (proprietary bus for control
registers) bus arbiter used on those SoCs. That commit arguably plugged
a hole in that scheduling was possible when panic() was intended, so
this is not really the only culprit. We are actually relying on this
behavior to pass a number of tests that specifically exercise that
register blocking is effective without taking down the whole system.

Due to our SoC integration all of those register access errors are
SErrors with the signature at the bottom.

Doug had tried to submit a patch series that allowed a given platform to
install custom abort handlers, similar to what ARM 32-bit permits, but
this got shot down:

https://lkml.org/lkml/2017/3/24/413

and this was eventually merged in this shape:

https://lore.kernel.org/patchwork/cover/775056/

I understand that such a SError is deemed catastrophic and
unrecoverable, but taking down the whole system for something we could
possibly resolve with a SIGSEGV provided the platform is known and hooks
are in place would be more desirable IMHO, otherwise we have nice DoS
lurking around and hard to debug systems in production, too.

As it stands today, I see no way to have a self hosted test case that
exercises that our GISB bus arbiter blocking works correctly because the
whole kernel is taken down when the test is successful :/

Thank you!

[   14.460690] SError Interrupt on CPU3, code 0xbf000002 -- SError
[   14.460695] CPU: 3 PID: 177 Comm: devmem Not tainted
5.6.0-rc7-g3893c2025fec #82
[   14.460696] Hardware name: BCX972160DV (DT)
[   14.460697] pstate: 60000000 (nZCv daif -PAN -UAO)
[   14.460699] pc : 00000000004087b0
[   14.460700] lr : 0000000000407b54
[   14.460701] sp : 0000007fea6fd740
[   14.460702] x29: 0000007fea6fd7e0 x28: 0000000000000000
[   14.460706] x27: 0000000000000000 x26: 0000000000000000
[   14.460709] x25: 0000000000000000 x24: 0000000000000000
[   14.460712] x23: 0000000000000000 x22: 0000000000000004
[   14.460714] x21: 00000000004cf000 x20: 0000007fea6fd918
[   14.460717] x19: 0000000000000029 x18: 0000000000050600
[   14.460720] x17: 00000000004cf408 x16: 0000007fba21b3d8
[   14.460723] x15: 000000000000013b x14: 0000000000000000
[   14.460726] x13: 0000000000000000 x12: 0000000000000007
[   14.460729] x11: 0000000000000008 x10: 0101010101010101
[   14.460731] x9 : 0000007fba25b1a8 x8 : 00000000000000de
[   14.460734] x7 : 1fffffffffffffff x6 : 0000007fba2999f0
[   14.460737] x5 : 0000000009902000 x4 : 0000000000000003
[   14.460739] x3 : 0000000000000001 x2 : 0000007fea6fdf71
[   14.460742] x1 : 0000000000000030 x0 : 0000000000000000
[   14.460745] Kernel panic - not syncing: Asynchronous SError Interrupt
[   14.460747] CPU: 3 PID: 177 Comm: devmem Not tainted
5.6.0-rc7-g3893c2025fec #82
[   14.460749] Hardware name: BCX972160DV (DT)
[   14.460750] Call trace:
[   14.460751]  dump_backtrace+0x0/0x1d8
[   14.460752]  show_stack+0x24/0x30
[   14.460753]  dump_stack+0xdc/0x14c
[   14.460754]  panic+0x13c/0x320
[   14.460755]  nmi_panic+0x50/0x70
[   14.460757]  arm64_serror_panic+0x74/0x80
[   14.460758]  do_serror+0xb4/0x158
[   14.460759]  el0_error_naked+0x14/0x1c
[   14.460781] SMP: stopping secondary CPUs
[   14.460782] Kernel Offset: disabled
[   14.460783] CPU features: 0x00002,24002004
[   14.460784] Memory Limit: none
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: SError handling vs. SIGSEGV
  2020-03-28  4:31 SError handling vs. SIGSEGV Florian Fainelli
@ 2020-03-28 16:43 ` Andrew Lunn
  2020-03-28 19:41   ` Florian Fainelli
  2020-03-30 11:30 ` James Morse
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2020-03-28 16:43 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Mark Rutland, Doug Berger, Scott Branden, Catalin Marinas,
	james.morse, Ray Jui, bcm-kernel-feedback-list, Will Deacon,
	Dave.Martin, linux-arm-kernel

> As it stands today, I see no way to have a self hosted test case that
> exercises that our GISB bus arbiter blocking works correctly because the
> whole kernel is taken down when the test is successful :/

Hi Florian

Isn't that just the new definition of test success :-)

Yes, your testing will be slower, you have to wait for the watchdog to
reboot your machine after each successful test.

      Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: SError handling vs. SIGSEGV
  2020-03-28 16:43 ` Andrew Lunn
@ 2020-03-28 19:41   ` Florian Fainelli
  2020-03-30 11:31     ` James Morse
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2020-03-28 19:41 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli
  Cc: Mark Rutland, Doug Berger, Scott Branden, Catalin Marinas,
	james.morse, Ray Jui, bcm-kernel-feedback-list, Will Deacon,
	Dave.Martin, linux-arm-kernel



On 3/28/2020 9:43 AM, Andrew Lunn wrote:
>> As it stands today, I see no way to have a self hosted test case that
>> exercises that our GISB bus arbiter blocking works correctly because the
>> whole kernel is taken down when the test is successful :/
> 
> Hi Florian
> 
> Isn't that just the new definition of test success :-)
> 
> Yes, your testing will be slower, you have to wait for the watchdog to
> reboot your machine after each successful test.

I would be fine with this behavior if the exception was taken from EL1,
but it is taken from EL0, killing the whole machine because of that
sounds like a great way to DDoS your machine without having much
valuable information to debug your problem if it exists.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: SError handling vs. SIGSEGV
  2020-03-28  4:31 SError handling vs. SIGSEGV Florian Fainelli
  2020-03-28 16:43 ` Andrew Lunn
@ 2020-03-30 11:30 ` James Morse
  2020-03-30 21:11   ` Florian Fainelli
  1 sibling, 1 reply; 8+ messages in thread
From: James Morse @ 2020-03-30 11:30 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Mark Rutland, Doug Berger, Scott Branden, Catalin Marinas,
	bcm-kernel-feedback-list, Ray Jui, Will Deacon, Dave.Martin,
	linux-arm-kernel

Hi Florian,

(I assume this is all on some pre-v8.2 system)

On 3/28/20 4:31 AM, Florian Fainelli wrote:
> Up until commit e4ba15debcfd27f60d43da940a58108783bff2a6 ("arm64:
> fix for bad_mode() handler to always result in panic") we had been
> getting SIGSEGV delivered to applications running on Broadcom STB
> platforms which access register holes or registers for which we have
> purposely blocked the access via the GISB (proprietary bus for control
> registers) bus arbiter used on those SoCs.

User-space has access to this? Not good.


> That commit arguably plugged
> a hole in that scheduling was possible when panic() was intended, so
> this is not really the only culprit.

> We are actually relying on this
> behavior to pass a number of tests that specifically exercise that
> register blocking is effective without taking down the whole system.

... but this isn't actually possible ...

The abort is asynchronous, the CPU may be doing something else when it arrives.
It may have taken an IRQ. Taking a (pre-v8.2) SError out of IRQ context has to
fatal.


> Due to our SoC integration all of those register access errors are
> SErrors with the signature at the bottom.

Do you trust user-space not to access them?

If not, don't give user-space access to those pages!


> Doug had tried to submit a patch series that allowed a given platform to
> install custom abort handlers, similar to what ARM 32-bit permits, but
> this got shot down:

For good reason. You cannot know that the abort was caused by your broken
hardware, and not an ECC error for the stack memory...

Getting this wrong leads to data corruption, and you have no reliable
information to base the decision on. (see below). The RAS extensions added the
CPU and system bits to improve this.


> I understand that such a SError is deemed catastrophic and
> unrecoverable, but taking down the whole system for something we could
> possibly resolve with a SIGSEGV provided the platform is known and hooks
> are in place would be more desirable IMHO, otherwise we have nice DoS
> lurking around and hard to debug systems in production, too.

SError is asynchronous. The ELR_EL1 value is meaningless.
The CPU can change exception level between the access that triggers the
external-abort being sent, and the result received. You can't even rely on
'while my process is running'.

SError can also be imprecise so you can't return from an SError exception. The
CPU is not obliged to put the world back in order before taking the exception.

precise/imprecise isn't commonly described. The arm-arm has: G1.3.4 "Definition
of a precise exception":
| An exception is described as precise when the exception handler receives the
| PE state and memory system state that is consistent with the PE having
| executed all of the instructions up to but not including the point in the
| instruction stream where the exception was taken, and none afterwards.
| Other than the SError interrupt all exceptions that are taken to AArch32 state
| are required to be precise. For each occurrence of an SError interrupt,
| whether the interrupt is precise or imprecise is IMPLEMENTATION DEFINED.


> As it stands today, I see no way to have a self hosted test case that
> exercises that our GISB bus arbiter blocking works correctly because the
> whole kernel is taken down when the test is successful :/

Sorry, it looks like your hardware people have given you something you can't
reliably work with.

Don't give user-space access to devices, they can take the system down.
Don't punch holes in page mappings, the CPU can't map anything smaller.
Don't respond with an abort unless you are prepared for the OS to die.




Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: SError handling vs. SIGSEGV
  2020-03-28 19:41   ` Florian Fainelli
@ 2020-03-30 11:31     ` James Morse
  2020-03-30 21:00       ` Florian Fainelli
  0 siblings, 1 reply; 8+ messages in thread
From: James Morse @ 2020-03-30 11:31 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Mark Rutland, Andrew Lunn, Florian Fainelli, Scott Branden,
	Catalin Marinas, Doug Berger, bcm-kernel-feedback-list, Ray Jui,
	Will Deacon, Dave.Martin, linux-arm-kernel

Hi guys,

On 3/28/20 7:41 PM, Florian Fainelli wrote:
> On 3/28/2020 9:43 AM, Andrew Lunn wrote:
>>> As it stands today, I see no way to have a self hosted test case that
>>> exercises that our GISB bus arbiter blocking works correctly because the
>>> whole kernel is taken down when the test is successful :/
>>
>> Hi Florian
>>
>> Isn't that just the new definition of test success :-)
>>
>> Yes, your testing will be slower, you have to wait for the watchdog to
>> reboot your machine after each successful test.

> I would be fine with this behavior if the exception was taken from EL1,
> but it is taken from EL0, killing the whole machine because of that
> sounds like a great way to DDoS your machine without having much
> valuable information to debug your problem if it exists.

And what if the exception was triggered by an access at EL0, but the CPU had
switched to EL1 before the response arrived, and the exception was taken?!


To 'handle' an SError you need it to be restartable at the CPU and you need to
know what caused it, so you can fix the error instead of it remaining latent.
Before the v8.2 RAS extensions you certainly don't have the CPU bits of this.


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: SError handling vs. SIGSEGV
  2020-03-30 11:31     ` James Morse
@ 2020-03-30 21:00       ` Florian Fainelli
  0 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-03-30 21:00 UTC (permalink / raw)
  To: James Morse
  Cc: Mark Rutland, Andrew Lunn, Florian Fainelli, Scott Branden,
	Catalin Marinas, Doug Berger, bcm-kernel-feedback-list, Ray Jui,
	Will Deacon, Dave.Martin, linux-arm-kernel



On 3/30/2020 4:31 AM, James Morse wrote:
> Hi guys,
> 
> On 3/28/20 7:41 PM, Florian Fainelli wrote:
>> On 3/28/2020 9:43 AM, Andrew Lunn wrote:
>>>> As it stands today, I see no way to have a self hosted test case that
>>>> exercises that our GISB bus arbiter blocking works correctly because the
>>>> whole kernel is taken down when the test is successful :/
>>>
>>> Hi Florian
>>>
>>> Isn't that just the new definition of test success :-)
>>>
>>> Yes, your testing will be slower, you have to wait for the watchdog to
>>> reboot your machine after each successful test.
> 
>> I would be fine with this behavior if the exception was taken from EL1,
>> but it is taken from EL0, killing the whole machine because of that
>> sounds like a great way to DDoS your machine without having much
>> valuable information to debug your problem if it exists.
> 
> And what if the exception was triggered by an access at EL0, but the CPU had
> switched to EL1 before the response arrived, and the exception was taken?!
> 
> 
> To 'handle' an SError you need it to be restartable at the CPU and you need to
> know what caused it, so you can fix the error instead of it remaining latent.
> Before the v8.2 RAS extensions you certainly don't have the CPU bits of this.

Good point, oh well, let's see how the HW guys can wire up their bus
better then :) Thank you
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: SError handling vs. SIGSEGV
  2020-03-30 11:30 ` James Morse
@ 2020-03-30 21:11   ` Florian Fainelli
  2020-03-31 16:59     ` James Morse
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2020-03-30 21:11 UTC (permalink / raw)
  To: James Morse
  Cc: Mark Rutland, Doug Berger, Scott Branden, Catalin Marinas,
	bcm-kernel-feedback-list, Ray Jui, Will Deacon, Dave.Martin,
	linux-arm-kernel



On 3/30/2020 4:30 AM, James Morse wrote:
> Hi Florian,
> 
> (I assume this is all on some pre-v8.2 system)

Cortex-A53, so yes.

> 
> On 3/28/20 4:31 AM, Florian Fainelli wrote:
>> Up until commit e4ba15debcfd27f60d43da940a58108783bff2a6 ("arm64:
>> fix for bad_mode() handler to always result in panic") we had been
>> getting SIGSEGV delivered to applications running on Broadcom STB
>> platforms which access register holes or registers for which we have
>> purposely blocked the access via the GISB (proprietary bus for control
>> registers) bus arbiter used on those SoCs.
> 
> User-space has access to this? Not good.

This is a test case that uses busybox's devmem (opens up /dev/mem), we
do not expect anyone in production to do that.

> 
> 
>> That commit arguably plugged
>> a hole in that scheduling was possible when panic() was intended, so
>> this is not really the only culprit.
> 
>> We are actually relying on this
>> behavior to pass a number of tests that specifically exercise that
>> register blocking is effective without taking down the whole system.
> 
> ... but this isn't actually possible ...
> 
> The abort is asynchronous, the CPU may be doing something else when it arrives.
> It may have taken an IRQ. Taking a (pre-v8.2) SError out of IRQ context has to
> fatal.

I suppose we have been lucky before in that we only ever saw the
offending application being killed.

> 
> 
>> Due to our SoC integration all of those register access errors are
>> SErrors with the signature at the bottom.
> 
> Do you trust user-space not to access them?
> 
> If not, don't give user-space access to those pages!

As I described, this is an intentional test case that exercises a bus
firewall feature, this is not necessarily representative of a real world
scenario.

> 
> 
>> Doug had tried to submit a patch series that allowed a given platform to
>> install custom abort handlers, similar to what ARM 32-bit permits, but
>> this got shot down:
> 
> For good reason. You cannot know that the abort was caused by your broken
> hardware, and not an ECC error for the stack memory...

ECC are separate syndromes which are actually known to the kernel.

> 
> Getting this wrong leads to data corruption, and you have no reliable
> information to base the decision on. (see below). The RAS extensions added the
> CPU and system bits to improve this.

The GISB bus does capture invalid accesses, that is the basis for this
driver: drivers/bus/brcmstb_gisb.c which is helpful in identifying a
block that is clock gated for instance. This would work too in the case
where a register address is intentionally blocked out.

> 
> 
>> I understand that such a SError is deemed catastrophic and
>> unrecoverable, but taking down the whole system for something we could
>> possibly resolve with a SIGSEGV provided the platform is known and hooks
>> are in place would be more desirable IMHO, otherwise we have nice DoS
>> lurking around and hard to debug systems in production, too.
> 
> SError is asynchronous. The ELR_EL1 value is meaningless.
> The CPU can change exception level between the access that triggers the
> external-abort being sent, and the result received. You can't even rely on
> 'while my process is running'.
> 
> SError can also be imprecise so you can't return from an SError exception. The
> CPU is not obliged to put the world back in order before taking the exception.
> 
> precise/imprecise isn't commonly described. The arm-arm has: G1.3.4 "Definition
> of a precise exception":
> | An exception is described as precise when the exception handler receives the
> | PE state and memory system state that is consistent with the PE having
> | executed all of the instructions up to but not including the point in the
> | instruction stream where the exception was taken, and none afterwards.
> | Other than the SError interrupt all exceptions that are taken to AArch32 state
> | are required to be precise. For each occurrence of an SError interrupt,
> | whether the interrupt is precise or imprecise is IMPLEMENTATION DEFINED.
> 
> 
>> As it stands today, I see no way to have a self hosted test case that
>> exercises that our GISB bus arbiter blocking works correctly because the
>> whole kernel is taken down when the test is successful :/
> 
> Sorry, it looks like your hardware people have given you something you can't
> reliably work with.

Yes indeed.

> 
> Don't give user-space access to devices, they can take the system down.
> Don't punch holes in page mappings, the CPU can't map anything smaller.
> Don't respond with an abort unless you are prepared for the OS to die.

That much is clear, thank you.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: SError handling vs. SIGSEGV
  2020-03-30 21:11   ` Florian Fainelli
@ 2020-03-31 16:59     ` James Morse
  0 siblings, 0 replies; 8+ messages in thread
From: James Morse @ 2020-03-31 16:59 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Mark Rutland, Doug Berger, Scott Branden, Catalin Marinas,
	bcm-kernel-feedback-list, Ray Jui, Will Deacon, Dave.Martin,
	linux-arm-kernel

Hi Florian,

Some more light on this one:

On 3/30/20 10:11 PM, Florian Fainelli wrote:
>>> Doug had tried to submit a patch series that allowed a given platform to
>>> install custom abort handlers, similar to what ARM 32-bit permits, but
>>> this got shot down:

>> For good reason. You cannot know that the abort was caused by your broken
>> hardware, and not an ECC error for the stack memory...

> ECC are separate syndromes which are actually known to the kernel.

Those extra external-abort FSC are for what the core was doing when it took the
abort. (e.g. page table walk)
You'd only get one of the ECC/parity values if the error was detected by the
core. But other components can detect an ECC error and return an abort, the core
doesn't know why this happened, it will show up with that 'catch all' value.

The v8.2 extensions change this by letting the component that triggered the
error describe why it did that.


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-31 17:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-28  4:31 SError handling vs. SIGSEGV Florian Fainelli
2020-03-28 16:43 ` Andrew Lunn
2020-03-28 19:41   ` Florian Fainelli
2020-03-30 11:31     ` James Morse
2020-03-30 21:00       ` Florian Fainelli
2020-03-30 11:30 ` James Morse
2020-03-30 21:11   ` Florian Fainelli
2020-03-31 16:59     ` James Morse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).