* [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used
@ 2016-04-06 21:27 Al Stone
2016-04-07 0:18 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Al Stone @ 2016-04-06 21:27 UTC (permalink / raw)
To: linux-kernel; +Cc: ahs3, Arnd Bergmann, Greg Kroah-Hartman
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 <jbastian@redhat.com>), 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.
Signed-off-by: Al Stone <ahs3@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/char/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index b272397..c532f62 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -587,7 +587,7 @@ config TELCLOCK
config DEVPORT
bool
- depends on !M68K
+ depends on !M68K && !ARM64
depends on ISA || PCI
default y
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used 2016-04-06 21:27 [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used Al Stone @ 2016-04-07 0:18 ` Greg Kroah-Hartman 2016-04-07 7:26 ` Geert Uytterhoeven 0 siblings, 1 reply; 6+ messages in thread From: Greg Kroah-Hartman @ 2016-04-07 0:18 UTC (permalink / raw) To: Al Stone; +Cc: linux-kernel, Arnd Bergmann On Wed, Apr 06, 2016 at 03:27:20PM -0600, Al Stone wrote: > 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 <jbastian@redhat.com>), 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. > > Signed-off-by: Al Stone <ahs3@redhat.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > drivers/char/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig > index b272397..c532f62 100644 > --- a/drivers/char/Kconfig > +++ b/drivers/char/Kconfig > @@ -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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used 2016-04-07 0:18 ` Greg Kroah-Hartman @ 2016-04-07 7:26 ` Geert Uytterhoeven 2016-04-07 15:56 ` Al Stone 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2016-04-07 7:26 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Al Stone, linux-kernel@vger.kernel.org, Arnd Bergmann On Thu, Apr 7, 2016 at 2:18 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Wed, Apr 06, 2016 at 03:27:20PM -0600, Al Stone wrote: >> 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 <jbastian@redhat.com>), 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); There are plenty of ways to crash a system as the root user... >> 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. >> >> Signed-off-by: Al Stone <ahs3@redhat.com> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> --- >> drivers/char/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig >> index b272397..c532f62 100644 >> --- a/drivers/char/Kconfig >> +++ b/drivers/char/Kconfig >> @@ -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? 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 -- geert@linux-m68k.org 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used 2016-04-07 7:26 ` Geert Uytterhoeven @ 2016-04-07 15:56 ` Al Stone 2016-04-09 2:21 ` Arnd Bergmann 2016-04-12 5:17 ` Jon Masters 0 siblings, 2 replies; 6+ messages in thread From: Al Stone @ 2016-04-07 15:56 UTC (permalink / raw) To: Geert Uytterhoeven, Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, Arnd Bergmann On 04/07/2016 01:26 AM, Geert Uytterhoeven wrote: > On Thu, Apr 7, 2016 at 2:18 AM, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: >> On Wed, Apr 06, 2016 at 03:27:20PM -0600, Al Stone wrote: >>> 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 <jbastian@redhat.com>), 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); > > There are plenty of ways to crash a system as the root user... Of course. This was just a new one. >>> 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. >>> >>> Signed-off-by: Al Stone <ahs3@redhat.com> >>> Cc: Arnd Bergmann <arnd@arndb.de> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> --- >>> drivers/char/Kconfig | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig >>> index b272397..c532f62 100644 >>> --- a/drivers/char/Kconfig >>> +++ b/drivers/char/Kconfig >>> @@ -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? 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... > 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 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. -- ciao, al ----------------------------------- Al Stone Software Engineer Red Hat, Inc. ahs3@redhat.com ----------------------------------- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used 2016-04-07 15:56 ` Al Stone @ 2016-04-09 2:21 ` Arnd Bergmann 2016-04-12 5:17 ` Jon Masters 1 sibling, 0 replies; 6+ messages in thread From: Arnd Bergmann @ 2016-04-09 2:21 UTC (permalink / raw) To: ahs3; +Cc: Geert Uytterhoeven, Greg Kroah-Hartman, linux-kernel@vger.kernel.org On Thursday 07 April 2016, Al Stone wrote: > >>> 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? > > 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... 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used 2016-04-07 15:56 ` Al Stone 2016-04-09 2:21 ` Arnd Bergmann @ 2016-04-12 5:17 ` Jon Masters 1 sibling, 0 replies; 6+ messages in thread From: Jon Masters @ 2016-04-12 5:17 UTC (permalink / raw) To: ahs3, Geert Uytterhoeven, Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, Arnd Bergmann On 04/07/2016 11:56 AM, Al Stone wrote: > config DEVPORT > bool > depends on ISA && PCI > default y > > That 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. -- Computer Architect ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-12 5:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-06 21:27 [PATCH] arm64: CONFIG_DEVPORT should not be used when PCI is being used Al Stone 2016-04-07 0:18 ` Greg Kroah-Hartman 2016-04-07 7:26 ` Geert Uytterhoeven 2016-04-07 15:56 ` Al Stone 2016-04-09 2:21 ` Arnd Bergmann 2016-04-12 5:17 ` Jon Masters
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).