arm64: CONFIG_DEVPORT should not be used when PCI is being used
PATCH 1/1 arm64: CONFIG_DEVPORT should not be used when PCI is being used
On arm64 systems, using /dev/port does not really make sense; this is historically used for other architectures to access ISA IO ports, which with any luck do not exist on arm64 platforms. With the following snippet of perl code (from Jeff Bastian <[email protected]>), we can reliably panic an arm64 system with PCI enabled:
#!/usr/bin/perl -w
# extracted from sensors-detect from lm_sensors
# to reproduce kernel crash when probing the
# Super-I/O ports
use Fcntl qw(:DEFAULT :seek);
sysopen(IOPORTS, "/dev/port", O_RDWR);
binmode(IOPORTS);
sysseek(IOPORTS, 0x2e, 0);
syswrite(IOPORTS, pack("C", 0x0d), 1);So, make sure CONFIG_DEVPORT cannot be set on arm64; it cannot really be used and it allows us to crash a kernel from user space.
--- drivers/char/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/Kconfig +1 -1
@@ -587,7 +587,7 @@ config TELCLOCK config DEVPORT bool - depends on !M68K + depends on !M68K && !ARM64 depends on ISA || PCI default y
Signature · 1 line
1.8.3.1
Greg Kroah-Hartman
maintainerQuoted 31 lines · Al Stone
On arm64 systems, using
/dev/portdoes not really make sense; this is historically used for other architectures to access ISA IO ports, which with any luck do not exist on arm64 platforms. With the following snippet of perl code (from Jeff Bastian <[email protected]>), we can reliably panic an arm64 system with PCI enabled:#!/usr/bin/perl -w # extracted from sensors-detect from lm_sensors # to reproduce kernel crash when probing the # Super-I/O ports use Fcntl qw(:DEFAULT :seek); sysopen(IOPORTS, "/dev/port", O_RDWR); binmode(IOPORTS); sysseek(IOPORTS, 0x2e, 0); syswrite(IOPORTS, pack("C", 0x0d), 1);So, make sure
CONFIG_DEVPORTcannot be set on arm64; it cannot really be used and it allows us to crash a kernel from user space.Signed-off-by Al Stone <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Greg Kroah-Hartman <[email protected]>--- drivers/char/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/char/Kconfig +0
-0
drivers/char/Kconfig config TELCLOCK · line 590
Quoted 5 lines
drivers/char/Kconfig +1
-1@@ -587,7 +587,7 @@ config TELCLOCK config DEVPORT bool - depends on !M68K + depends on !M68K && !ARM64
Why not fix the real bug here, it's odd that only these two arches need this disabled, don't you agree?
thanks,
greg k-h
Quoted 7 lines · Greg Kroah-Hartman
Quoted 6 lines · Al Stone
On arm64 systems, using
/dev/portdoes not really make sense; this is historically used for other architectures to access ISA IO ports, which with any luck do not exist on arm64 platforms. With the following snippet of perl code (from Jeff Bastian <[email protected]>), we can reliably panic an arm64 system with PCI enabled:
Quoted 9 lines
Quoted 9 lines
#!/usr/bin/perl -w # extracted from sensors-detect from lm_sensors # to reproduce kernel crash when probing the # Super-I/O ports use Fcntl qw(:DEFAULT :seek); sysopen(IOPORTS, "/dev/port", O_RDWR); binmode(IOPORTS); sysseek(IOPORTS, 0x2e, 0); syswrite(IOPORTS, pack("C", 0x0d), 1);
There are plenty of ways to crash a system as the root user...
Quoted 16 lines
Quoted 16 lines
So, make sure
CONFIG_DEVPORTcannot be set on arm64; it cannot really be used and it allows us to crash a kernel from user space.Signed-off-by Al Stone <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Greg Kroah-Hartman <[email protected]>--- drivers/char/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/char/Kconfig +0
-0@@ -587,7 +587,7 @@ config TELCLOCK
Quoted 7 lines
drivers/char/Kconfig config TELCLOCK · line 590
Quoted 4 lines
drivers/char/Kconfig +1
-1config DEVPORT bool - depends on !M68K + depends on !M68K && !ARM64Why not fix the real bug here, it's odd that only these two arches need this disabled, don't you agree?
In fact even the !M68K dependency is odd.
The logic seems to originate from commit 153dcc54df826d2f ("[PATCH] mem driver:
fix conditional on isa i/o support"), which accidentally changed an "||" into an "&&".
Will send a patch later...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Quoted 19 lines · Geert Uytterhoeven
Quoted 7 lines · Greg Kroah-Hartman
Quoted 6 lines · Al Stone
On arm64 systems, using
/dev/portdoes not really make sense; this is historically used for other architectures to access ISA IO ports, which with any luck do not exist on arm64 platforms. With the following snippet of perl code (from Jeff Bastian <[email protected]>), we can reliably panic an arm64 system with PCI enabled:Quoted 9 lines
Quoted 9 lines
#!/usr/bin/perl -w # extracted from sensors-detect from lm_sensors # to reproduce kernel crash when probing the # Super-I/O ports use Fcntl qw(:DEFAULT :seek); sysopen(IOPORTS, "/dev/port", O_RDWR); binmode(IOPORTS); sysseek(IOPORTS, 0x2e, 0); syswrite(IOPORTS, pack("C", 0x0d), 1);
Quoted 1 line
There are plenty of ways to crash a system as the root user...
Of course. This was just a new one.
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
So, make sure
CONFIG_DEVPORTcannot be set on arm64; it cannot really be used and it allows us to crash a kernel from user space.Signed-off-by Al Stone <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Greg Kroah-Hartman <[email protected]>--- drivers/char/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/char/Kconfig +0
-0@@ -587,7 +587,7 @@ config TELCLOCK
Quoted 7 lines
Quoted 7 lines
drivers/char/Kconfig config TELCLOCK · line 590
Quoted 4 lines
drivers/char/Kconfig +1
-1config DEVPORT bool - depends on !M68K + depends on !M68K && !ARM64Why not fix the real bug here, it's odd that only these two arches need this disabled, don't you agree?
Agreed. It does seem odd. I'm not sure I understand which bug you're thinking is the real one, though -- that DEVPORT should be disabled in all places that don't have ISA or that arm64 needs to have /dev/port work properly? Or perhaps I missed something else entirely...
Quoted 10 lines
In fact even the !M68K dependency is odd.
The logic seems to originate from commit153dcc54df826d2f("[PATCH] mem driver:
fix conditional on isa i/o support"), which accidentally changed an "||" into an "&&".Will send a patch later...
Gr{oetje,eeting}s,
Geert
Aha. I missed that bit. Sorry about that. So something like this instead:
config DEVPORT
bool depends on ISA && PCI default y
That makes more sense. Thanks.
Signature · 8 lines
ciao, al ----------------------------------- Al Stone Software Engineer Red Hat, Inc. [email protected] -----------------------------------
Arnd Bergmann
maintainerQuoted 5 lines · Al Stone
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
config DEVPORT
booldrivers/char/Kconfig +1
-1- depends on !M68K + depends on !M68K && !ARM64
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Why not fix the real bug here, it's odd that only these two arches need this disabled, don't you agree?
Agreed. It does seem odd. I'm not sure I understand which bug you're thinking is the real one, though -- that DEVPORT should be disabled in all places that don't have ISA or that arm64 needs to have
/dev/portwork properly? Or perhaps I missed something else entirely...
We've had a similar problem recently with ISA drivers crashing when no PCI host registers itself for the first 0x1000 I/O port addresses.
I think both the request_resource() function and /dev/ioport should be changed to interface with the dynamic registration of I/O port ranges so they only ever allow access on ports that are mapped into virtual memory.
Arnd
Quoted 6 lines · Al Stone
config DEVPORT
bool depends on ISA && PCI default yThat makes more sense. Thanks.
I think Itanium does IO ports but not ISA. Probably best to just turn on IO ports on the three architectures that use them in that case.
Jon.
Signature · 1 line
Computer Architect