CONFIG_CPU_SW_DOMAIN_PAN breakage on ARM11 MPCore

Hi,

I just wasted a few hours debugging user space hangs after updating my
CNS3xxx device to Linux 4.4 - until I found this:
http://www.spinics.net/lists/arm-kernel/msg450888.html

I haven't seen any follow-up patches since that discussion in October,
and this patch resolves the issue in my test. Could you guys please get
this merged and pushed to stable to avoid wasting other people's time as
well?

If you want to solve the issue in a different way, I can also test
patches for you.

Thanks,

- Felix

On Tue, Jan 19, 2016 at 12:14:59AM +0100, Felix Fietkau wrote:

I just wasted a few hours debugging user space hangs after updating my
CNS3xxx device to Linux 4.4 - until I found this:
http://www.spinics.net/lists/arm-kernel/msg450888.html

Sorry about that.  What you've found is an investigatory patch, not
a fix for it.

I haven't seen any follow-up patches since that discussion in October,
and this patch resolves the issue in my test.

If you read the remainder of the thread, you'll notice that there was
discussion about the problem and that isn't a proper fix.

Could you guys please get this merged and pushed to stable to avoid
wasting other people's time as well?

No one understands what's going on with ARM11 MPcore (not even ARM Ltd
people).  We certainly do not want to add the expense of writing to
the DACR each and every time we want to do a TLB flush, and as Will
said at the end of the thread, it should be predicated on ARM11 MPcore.
We don't know which revisions of ARM11 MPcore are affected.

Please can you provide your boot messages so we can see what revision
11MPcore you have in your device.  Thanks.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

On 2016-01-19 10:38, Russell King - ARM Linux wrote:

On Tue, Jan 19, 2016 at 12:14:59AM +0100, Felix Fietkau wrote:

quoted

I just wasted a few hours debugging user space hangs after updating my
CNS3xxx device to Linux 4.4 - until I found this:
http://www.spinics.net/lists/arm-kernel/msg450888.html

Sorry about that.  What you've found is an investigatory patch, not
a fix for it.

quoted

I haven't seen any follow-up patches since that discussion in October,
and this patch resolves the issue in my test.

If you read the remainder of the thread, you'll notice that there was
discussion about the problem and that isn't a proper fix.

quoted

Could you guys please get this merged and pushed to stable to avoid
wasting other people's time as well?

No one understands what's going on with ARM11 MPcore (not even ARM Ltd
people).  We certainly do not want to add the expense of writing to
the DACR each and every time we want to do a TLB flush, and as Will
said at the end of the thread, it should be predicated on ARM11 MPcore.
We don't know which revisions of ARM11 MPcore are affected.

Please can you provide your boot messages so we can see what revision
11MPcore you have in your device.  Thanks.

[    0.000000] Booting Linux on physical CPU 0x900
[    0.000000] Linux version 4.4.0 (nbd at nf.lan) (gcc version 5.2.0 (OpenWrt GCC 5.2.0 r48326) ) #8 SMP Tue Jan 19 00:06:54 CET 2016
[    0.000000] CPU: ARMv6-compatible processor [410fb024] revision 4 (ARMv7), cr=00c5787d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] Machine: Gateworks Corporation Laguna Platform
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] PERCPU: Embedded 12 pages/cpu @c7edb000 s16512 r8192 d24448 u49152
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
[    0.000000] Kernel command line: console=ttyS0,115200 root=/dev/mtdblock3 rootfstype=squashfs,jffs2 noinitrd init=/etc/preinit
[    0.000000] PID hash table entries: 512 (order: -1, 2048 bytes)
[    0.000000] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
[    0.000000] Memory: 119144K/131072K available (3510K kernel code, 100K rwdata, 936K rodata, 5896K init, 202K bss, 11928K reserved, 0K cma-reserved)
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
[    0.000000]     vmalloc : 0xc8800000 - 0xff800000   ( 880 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xc8000000   ( 128 MB)
[    0.000000]     modules : 0xbf000000 - 0xc0000000   (  16 MB)
[    0.000000]       .text : 0xc0008000 - 0xc045fe6c   (4448 kB)
[    0.000000]       .init : 0xc0460000 - 0xc0a22000   (5896 kB)
[    0.000000]       .data : 0xc0a22000 - 0xc0a3b280   ( 101 kB)
[    0.000000]        .bss : 0xc0a3b280 - 0xc0a6daac   ( 203 kB)
[    0.000000] Hierarchical RCU implementation.
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] clocksource: freerun: mask: 0xffffffffffff max_cycles: 0x179dd7f66, max_idle_ns: 28210892933900 ns
[    0.000000] smp_twd: clock not found -2
[    0.000000] sched_clock: 32 bits at 100 Hz, resolution 10000000ns, wraps every 21474836475000000ns
[    0.130000] console [ttyS0] enabled
[    0.140000] Calibrating local timer... 299.92MHz.
[    0.200000] Calibrating delay loop... 238.38 BogoMIPS (lpj=1191936)
[    0.270000] pid_max: default: 32768 minimum: 301
[    0.270000] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.280000] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.290000] CPU: Testing write buffer coherency: ok
[    0.290000] Setting up static identity map for 0x20008280 - 0x200082b8
[    0.370000] Brought up 2 CPUs
[    0.370000] SMP: Total of 2 processors activated (478.00 BogoMIPS).
[    0.380000] VFP support v0.3: implementor 41 architecture 1 part 20 variant b rev 4
[    0.390000] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns

173 more lines

On Tuesday 19 January 2016 10:53:26 Felix Fietkau wrote:

root at OpenWrt:~# cat /proc/cpuinfo 
processor       : 0
model name      : ARMv6-compatible processor rev 4 (v6l)
BogoMIPS        : 238.38
Features        : half thumb fastmult vfp edsp java tls 
CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x0
CPU part        : 0xb02
CPU revision    : 4

processor       : 1
model name      : ARMv6-compatible processor rev 4 (v6l)
BogoMIPS        : 239.61
Features        : half thumb fastmult vfp edsp java tls 
CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x0
CPU part        : 0xb02
CPU revision    : 4

I guess this means you run with the SMP patches from OpenWRT,
while upstream only supports uniprocessor mode and presumably doesn't
have this problem, right?

I see that Oxnas (supported in OpenWRT but not upstream) has the
slightly newer ARM11mpcore variant 0 / revision 5 ID. Is it easy for
you to test if this has the same problem?

Interestingly, the documentation at http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0360f/I65012.html
lists variant:0/revision:4 as the reported values for both r2p0 and r1p0
and it does not list any core having variant:0/revision:5. 

	Arnd

On 2016-01-19 16:27, Arnd Bergmann wrote:

On Tuesday 19 January 2016 10:53:26 Felix Fietkau wrote:

quoted

root at OpenWrt:~# cat /proc/cpuinfo 
processor       : 0
model name      : ARMv6-compatible processor rev 4 (v6l)
BogoMIPS        : 238.38
Features        : half thumb fastmult vfp edsp java tls 
CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x0
CPU part        : 0xb02
CPU revision    : 4

processor       : 1
model name      : ARMv6-compatible processor rev 4 (v6l)
BogoMIPS        : 239.61
Features        : half thumb fastmult vfp edsp java tls 
CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x0
CPU part        : 0xb02
CPU revision    : 4

I guess this means you run with the SMP patches from OpenWRT,
while upstream only supports uniprocessor mode and presumably doesn't
have this problem, right?

I tested without SMP and ran into the same user space hangs/crashes.

I see that Oxnas (supported in OpenWRT but not upstream) has the
slightly newer ARM11mpcore variant 0 / revision 5 ID. Is it easy for
you to test if this has the same problem?

I already spoke to Daniel (who maintains that target), and it is equally
affected.

Interestingly, the documentation at http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0360f/I65012.html
lists variant:0/revision:4 as the reported values for both r2p0 and r1p0
and it does not list any core having variant:0/revision:5. 

Heh, interesting.

- Felix

On Tue, Jan 19, 2016 at 04:27:49PM +0100, Arnd Bergmann wrote:

I guess this means you run with the SMP patches from OpenWRT,
while upstream only supports uniprocessor mode and presumably doesn't
have this problem, right?

It's not got much to do with SMP vs UP mode - it seems to be a CPU
core bug, though really we have no real idea what's going on here
as Will was unable to easily get hold of the information within
ARM Ltd to properly diagnose this (it's been "archived", and which
is why it remains unfixed - we've a deficit of information.)

However, the SMP vs UP mode thing does have an effect on the fix
too - if we have MPcore systems operating in UP mode, we're going
to need a much more complex and hideous fix - we're likely going
to need to out-of-line _all_ the TLB flushing which is going to
be nasty for the vast majority not affected by this. :(

I see that Oxnas (supported in OpenWRT but not upstream) has the
slightly newer ARM11mpcore variant 0 / revision 5 ID. Is it easy for
you to test if this has the same problem?

LinusW's failing case is a MPcore with a MIDR of 0x410fb020 (r0p0).
Felix's case is 0x410fb024.  Even if we had implemented a work-around
for Linus' case, without any further information, we'd have assumed
that only r0p0 is affected, which would not have caught Felix's case.

Interestingly, the documentation at
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0360f/I65012.html
lists variant:0/revision:4 as the reported values for both r2p0 and r1p0
and it does not list any core having variant:0/revision:5. 

I wouldn't read too much into that - I suspect the r2p0 document
wasn't updated properly.  That's why I prefer to go by a definitive
list of variant/revisions to rNpN when listed, or by discovering
what's actually out in the field compared to what it should contain.

It is equally possible that r2p0 and r1p0 do indeed share the same
variant/revision and someone forgot to change the hardware.

Does it make much difference though - we now know that variant 0/
revision 0 and variant 0/revision 4 are both affected, but we still
don't know whether variant 0/revision 5 is, or whether all 11MPcores
are affected.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

On Tue, Jan 19, 2016 at 04:23:28PM +0000, Russell King - ARM Linux wrote:

However, the SMP vs UP mode thing does have an effect on the fix
too - if we have MPcore systems operating in UP mode, we're going
to need a much more complex and hideous fix - we're likely going
to need to out-of-line _all_ the TLB flushing which is going to
be nasty for the vast majority not affected by this. :(

Having thought about this some more, I'm coming to the conclusion that
the only sane solution here is to change the help text for SW_PAN such
that if you want to run a kernel on ARM11 MPcore, you must disable
SW_PAN.

Unless that approach is taken, we're into a rewrite the ARM TLB flushing
(as mentioned above) and I really don't want to do that just for the
sake of one relatively rare early SMP CPU.

For those who think we can simply apply my patch, consider the CNS3xxx
situation, which is not a SMP system in mainline kernels, but uses ARM11
MPcore CPUs (and thus fails when SMP is disabled, even with my patch.)

So I'm going to suggest that this option's help text is changed to:

config CPU_SW_DOMAIN_PAN
	bool "Enable use of CPU domains to implement privileged no-access"
	depends on MMU && !ARM_LPAE
	default y
	help
	  Increase kernel security by ensuring that normal kernel accesses
	  are unable to access userspace addresses.  This can help prevent
	  use-after-free bugs becoming an exploitable privilege escalation
	  by ensuring that magic values (such as LIST_POISON) will always
	  fault when dereferenced.

	  Note: This option is incompatible with ARM11 MPcore and must not
	  be used with kernels which are to run on this CPU, whether in SMP
	  or UP mode.

	  CPUs with low-vector mappings use a best-efforts implementation.
	  Their lower 1MB needs to remain accessible for the vectors, but
	  the remainder of userspace will become appropriately inaccessible.

Unfortunately, that's still going to lead to people hitting this, and
possibly wasting a long time debugging it needlessly - but I don't
have any better solution for this.

-- 

3 more lines

On 2016-01-20 20:57, Russell King - ARM Linux wrote:

On Tue, Jan 19, 2016 at 04:23:28PM +0000, Russell King - ARM Linux wrote:

quoted

However, the SMP vs UP mode thing does have an effect on the fix
too - if we have MPcore systems operating in UP mode, we're going
to need a much more complex and hideous fix - we're likely going
to need to out-of-line _all_ the TLB flushing which is going to
be nasty for the vast majority not affected by this. :(

Having thought about this some more, I'm coming to the conclusion that
the only sane solution here is to change the help text for SW_PAN such
that if you want to run a kernel on ARM11 MPcore, you must disable
SW_PAN.

Unless that approach is taken, we're into a rewrite the ARM TLB flushing
(as mentioned above) and I really don't want to do that just for the
sake of one relatively rare early SMP CPU.

For those who think we can simply apply my patch, consider the CNS3xxx
situation, which is not a SMP system in mainline kernels, but uses ARM11
MPcore CPUs (and thus fails when SMP is disabled, even with my patch.)

So I'm going to suggest that this option's help text is changed to:

config CPU_SW_DOMAIN_PAN
	bool "Enable use of CPU domains to implement privileged no-access"
	depends on MMU && !ARM_LPAE
	default y
	help
	  Increase kernel security by ensuring that normal kernel accesses
	  are unable to access userspace addresses.  This can help prevent
	  use-after-free bugs becoming an exploitable privilege escalation
	  by ensuring that magic values (such as LIST_POISON) will always
	  fault when dereferenced.

	  Note: This option is incompatible with ARM11 MPcore and must not
	  be used with kernels which are to run on this CPU, whether in SMP
	  or UP mode.

	  CPUs with low-vector mappings use a best-efforts implementation.
	  Their lower 1MB needs to remain accessible for the vectors, but
	  the remainder of userspace will become appropriately inaccessible.

4 more quoted lines

We should at least add a dependency to disable this when support for a
known ARM11 MPCore platform is selected. Maybe add a CPU_MPCORE bool for
this.

- Felix

On Wednesday 20 January 2016 21:06:01 Felix Fietkau wrote:

quoted

config CPU_SW_DOMAIN_PAN
      bool "Enable use of CPU domains to implement privileged no-access"
      depends on MMU && !ARM_LPAE
      default y
      help
        Increase kernel security by ensuring that normal kernel accesses
        are unable to access userspace addresses.  This can help prevent
        use-after-free bugs becoming an exploitable privilege escalation
        by ensuring that magic values (such as LIST_POISON) will always
        fault when dereferenced.

        Note: This option is incompatible with ARM11 MPcore and must not
        be used with kernels which are to run on this CPU, whether in SMP
        or UP mode.

        CPUs with low-vector mappings use a best-efforts implementation.
        Their lower 1MB needs to remain accessible for the vectors, but
        the remainder of userspace will become appropriately inaccessible.

Unfortunately, that's still going to lead to people hitting this, and
possibly wasting a long time debugging it needlessly - but I don't
have any better solution for this.

We should at least add a dependency to disable this when support for a
known ARM11 MPCore platform is selected. Maybe add a CPU_MPCORE bool for
this.

Just depending on (!ARCH_CNS3XXX && !REALVIEW_EB_ARM11MP &&
!MACH_REALVIEW_PB11MP) would be sufficient technically, but adding a
CPU_ARM11MPCORE seems a little nicer.

The downside is that it departs from the the idea that starting with
ARMv6 we only have configuration symbols for the architecture level
(CPU_V6, CPU_V7), but we also have a CPU_PJ4 symbol that breaks this
rule.

If we add the CPU_ARM11MPCORE symbol, we may also want to update
CONFIG_SMP to depend on (CPU_ARM11MPCORE || CPU_V7) instead of CPU_V6K,
and we can force-enable SMP_ON_UP whenever (CPU_V6 && !CPU_ARM11MPCORE)

	Arnd