diff mbox

arm64: defconfig: disable CONFIG_DEVMEM

Message ID 20170113113734.16524-1-leif.lindholm@linaro.org
State New
Headers show

Commit Message

Leif Lindholm Jan. 13, 2017, 11:37 a.m. UTC
/dev/mem is the opposite of what an operating system is for.
Additionally, on arm* it opens up for denial-of-service attacks from
userspace. So leave it disabled by default, requiring people who need
it to enable it explicitly.

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

-- 
2.10.2


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

Comments

Ard Biesheuvel Jan. 13, 2017, 11:38 a.m. UTC | #1
On 13 January 2017 at 11:37, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> /dev/mem is the opposite of what an operating system is for.

> Additionally, on arm* it opens up for denial-of-service attacks from

> userspace. So leave it disabled by default, requiring people who need

> it to enable it explicitly.

>

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>


Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


> ---

>  arch/arm64/configs/defconfig | 1 +

>  1 file changed, 1 insertion(+)

>

> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig

> index 33b744d..55ba73a 100644

> --- a/arch/arm64/configs/defconfig

> +++ b/arch/arm64/configs/defconfig

> @@ -210,6 +210,7 @@ CONFIG_INPUT_HISI_POWERKEY=y

>  # CONFIG_SERIO_SERPORT is not set

>  CONFIG_SERIO_AMBAKMI=y

>  CONFIG_LEGACY_PTY_COUNT=16

> +# CONFIG_DEVMEM is not set

>  CONFIG_SERIAL_8250=y

>  CONFIG_SERIAL_8250_CONSOLE=y

>  CONFIG_SERIAL_8250_EXTENDED=y

> --

> 2.10.2

>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Mark Rutland Jan. 13, 2017, 11:40 a.m. UTC | #2
On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:
> /dev/mem is the opposite of what an operating system is for.

> Additionally, on arm* it opens up for denial-of-service attacks from

> userspace. So leave it disabled by default, requiring people who need

> it to enable it explicitly.

> 

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>


Generally I'd agree that the use of /dev/mem is a bad idea (TM), so I'm
surprised we have it in our defconfig.

FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>


Mark.

> ---

>  arch/arm64/configs/defconfig | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig

> index 33b744d..55ba73a 100644

> --- a/arch/arm64/configs/defconfig

> +++ b/arch/arm64/configs/defconfig

> @@ -210,6 +210,7 @@ CONFIG_INPUT_HISI_POWERKEY=y

>  # CONFIG_SERIO_SERPORT is not set

>  CONFIG_SERIO_AMBAKMI=y

>  CONFIG_LEGACY_PTY_COUNT=16

> +# CONFIG_DEVMEM is not set

>  CONFIG_SERIAL_8250=y

>  CONFIG_SERIAL_8250_CONSOLE=y

>  CONFIG_SERIAL_8250_EXTENDED=y

> -- 

> 2.10.2

> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Will Deacon Jan. 13, 2017, 11:48 a.m. UTC | #3
On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:
> /dev/mem is the opposite of what an operating system is for.

> Additionally, on arm* it opens up for denial-of-service attacks from

> userspace. So leave it disabled by default, requiring people who need

> it to enable it explicitly.


I really like the idea, but are we sure that nothing common breaks without
this? For example, does Debian still boot nicely with this patch applied?

Just trying to get a feel for how much userspace this has seen (particularly
on ACPI-based systems, which I seem to remember like poking about in here).

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Leif Lindholm Jan. 15, 2017, 12:42 p.m. UTC | #4
On Fri, Jan 13, 2017 at 11:48:01AM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:

> > /dev/mem is the opposite of what an operating system is for.

> > Additionally, on arm* it opens up for denial-of-service attacks from

> > userspace. So leave it disabled by default, requiring people who need

> > it to enable it explicitly.

> 

> I really like the idea, but are we sure that nothing common breaks without

> this? For example, does Debian still boot nicely with this patch applied?


Getting distros to not have to shop around for config fragments in
order to be able to a stable system is one of my main purposes of this
patch.

Since Debian just published a 4.9 kernel, I gave that a spin (both DT
and ACPI). No issues.

> Just trying to get a feel for how much userspace this has seen (particularly

> on ACPI-based systems, which I seem to remember like poking about in here).


There are various tools that let you figure out various things about
the system (a bit like running dtc on a dump of the active
device-tree), but nothing actually used for booting a system (much
like dtc).

/dev/mem has been used for things like dmidecode and acpidump in the
past, but we fixed those tools back in 2015 to use /sys interfaces
instead of blindly groping around and hoping for the best.

Stretch version of dmidecode operates as expected on both DT and ACPI
boot, and acpidump does in the ACPI case (no tables in /sys otherwise).

There may be other tools that will also break if not implementing
support for access via /sys, but none critical for system operation
(and currently a denial-of-service waiting to happen anyway).

Systemd will try to access /dev/mem to extract boot timestamp stuff,
but handles a failure gracefully (i.e. not even a warning message).

On a side note, comparing the resulting configs, there is a
semi-broken config dependency in lib/Kconfig.debug, meaning
CONFIG_*STRICT_DEVMEM get set even if CONFIG_DEVMEM is not. 
But I'll send that out as a separate patch.

/
    Leif

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Leif Lindholm Jan. 16, 2017, 1:40 p.m. UTC | #5
On Sun, Jan 15, 2017 at 12:42:54PM +0000, Leif Lindholm wrote:
> On a side note, comparing the resulting configs, there is a

> semi-broken config dependency in lib/Kconfig.debug, meaning

> CONFIG_*STRICT_DEVMEM get set even if CONFIG_DEVMEM is not. 

> But I'll send that out as a separate patch.


Ah - never mind, that one has already been fixed upstream for 4.10rc.

/
    Leif

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Leif Lindholm Jan. 18, 2017, 5:58 p.m. UTC | #6
On 15 January 2017 at 12:42, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Jan 13, 2017 at 11:48:01AM +0000, Will Deacon wrote:

>> On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:

>> > /dev/mem is the opposite of what an operating system is for.

>> > Additionally, on arm* it opens up for denial-of-service attacks from

>> > userspace. So leave it disabled by default, requiring people who need

>> > it to enable it explicitly.

>>

>> I really like the idea, but are we sure that nothing common breaks without

>> this? For example, does Debian still boot nicely with this patch applied?

>

> Getting distros to not have to shop around for config fragments in

> order to be able to a stable system is one of my main purposes of this

> patch.

>

> Since Debian just published a 4.9 kernel, I gave that a spin (both DT

> and ACPI). No issues.


Will, any comments?

/
    Leif

>> Just trying to get a feel for how much userspace this has seen (particularly

>> on ACPI-based systems, which I seem to remember like poking about in here).

>

> There are various tools that let you figure out various things about

> the system (a bit like running dtc on a dump of the active

> device-tree), but nothing actually used for booting a system (much

> like dtc).

>

> /dev/mem has been used for things like dmidecode and acpidump in the

> past, but we fixed those tools back in 2015 to use /sys interfaces

> instead of blindly groping around and hoping for the best.

>

> Stretch version of dmidecode operates as expected on both DT and ACPI

> boot, and acpidump does in the ACPI case (no tables in /sys otherwise).

>

> There may be other tools that will also break if not implementing

> support for access via /sys, but none critical for system operation

> (and currently a denial-of-service waiting to happen anyway).

>

> Systemd will try to access /dev/mem to extract boot timestamp stuff,

> but handles a failure gracefully (i.e. not even a warning message).


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Will Deacon Jan. 19, 2017, 9:49 a.m. UTC | #7
On Wed, Jan 18, 2017 at 05:58:05PM +0000, Leif Lindholm wrote:
> On 15 January 2017 at 12:42, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > On Fri, Jan 13, 2017 at 11:48:01AM +0000, Will Deacon wrote:

> >> On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:

> >> > /dev/mem is the opposite of what an operating system is for.

> >> > Additionally, on arm* it opens up for denial-of-service attacks from

> >> > userspace. So leave it disabled by default, requiring people who need

> >> > it to enable it explicitly.

> >>

> >> I really like the idea, but are we sure that nothing common breaks without

> >> this? For example, does Debian still boot nicely with this patch applied?

> >

> > Getting distros to not have to shop around for config fragments in

> > order to be able to a stable system is one of my main purposes of this

> > patch.

> >

> > Since Debian just published a 4.9 kernel, I gave that a spin (both DT

> > and ACPI). No issues.

> 

> Will, any comments?


Nope; fine by me. arm-soc usually pick these up.

FWIW, I did do a Debian CodeSearch for /dev/mem and there were more hits
than I was hoping for. However, much of the code looked either:

 (a) Broken anyway
 (b) Not relevant to arm64
 (c) /dev/mem was just a string in a comment or something

There are a couple of crazy projects that appear to use /dev/mem to
implement a heap, but I really couldn't figure out how that worked.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Leif Lindholm Jan. 20, 2017, 2:42 p.m. UTC | #8
On Thu, Jan 19, 2017 at 09:49:17AM +0000, Will Deacon wrote:
> On Wed, Jan 18, 2017 at 05:58:05PM +0000, Leif Lindholm wrote:

> > On 15 January 2017 at 12:42, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > > On Fri, Jan 13, 2017 at 11:48:01AM +0000, Will Deacon wrote:

> > >> On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:

> > >> > /dev/mem is the opposite of what an operating system is for.

> > >> > Additionally, on arm* it opens up for denial-of-service attacks from

> > >> > userspace. So leave it disabled by default, requiring people who need

> > >> > it to enable it explicitly.

> > >>

> > >> I really like the idea, but are we sure that nothing common breaks without

> > >> this? For example, does Debian still boot nicely with this patch applied?

> > >

> > > Getting distros to not have to shop around for config fragments in

> > > order to be able to a stable system is one of my main purposes of this

> > > patch.

> > >

> > > Since Debian just published a 4.9 kernel, I gave that a spin (both DT

> > > and ACPI). No issues.

> > 

> > Will, any comments?

> 

> Nope; fine by me. arm-soc usually pick these up.


Arnd, Olof?

> FWIW, I did do a Debian CodeSearch for /dev/mem and there were more hits

> than I was hoping for. However, much of the code looked either:

> 

>  (a) Broken anyway

>  (b) Not relevant to arm64

>  (c) /dev/mem was just a string in a comment or something

> 

> There are a couple of crazy projects that appear to use /dev/mem to

> implement a heap, but I really couldn't figure out how that worked.


Yeah, I noticed those too.
I don't think they do.

/
    Leif

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

Patch

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 33b744d..55ba73a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -210,6 +210,7 @@  CONFIG_INPUT_HISI_POWERKEY=y
 # CONFIG_SERIO_SERPORT is not set
 CONFIG_SERIO_AMBAKMI=y
 CONFIG_LEGACY_PTY_COUNT=16
+# CONFIG_DEVMEM is not set
 CONFIG_SERIAL_8250=y
 CONFIG_SERIAL_8250_CONSOLE=y
 CONFIG_SERIAL_8250_EXTENDED=y