| Message ID | 20170113113734.16524-1-leif.lindholm@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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 >
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
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
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
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).
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
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
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
/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(+)