lkml.org 
[lkml]   [2019]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH V34 20/29] x86/mmiotrace: Lock down the testmmiotrace module
    On Fri, Jun 21, 2019 at 05:03:49PM -0700, Matthew Garrett wrote:
    > From: David Howells <dhowells@redhat.com>
    >
    > The testmmiotrace module shouldn't be permitted when the kernel is locked
    > down as it can be used to arbitrarily read and write MMIO space. This is
    > a runtime check rather than buildtime in order to allow configurations
    > where the same kernel may be run in both locked down or permissive modes
    > depending on local policy.
    >
    > Suggested-by: Thomas Gleixner <tglx@linutronix.de>
    > Signed-off-by: David Howells <dhowells@redhat.com

    Reviewed-by: Kees Cook <keescook@chromium.org>

    -Kees

    > Signed-off-by: Matthew Garrett <mjg59@google.com>
    > cc: Thomas Gleixner <tglx@linutronix.de>
    > cc: Steven Rostedt <rostedt@goodmis.org>
    > cc: Ingo Molnar <mingo@kernel.org>
    > cc: "H. Peter Anvin" <hpa@zytor.com>
    > cc: x86@kernel.org
    > ---
    > arch/x86/mm/testmmiotrace.c | 5 +++++
    > include/linux/security.h | 1 +
    > security/lockdown/lockdown.c | 1 +
    > 3 files changed, 7 insertions(+)
    >
    > diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
    > index f6ae6830b341..6b9486baa2e9 100644
    > --- a/arch/x86/mm/testmmiotrace.c
    > +++ b/arch/x86/mm/testmmiotrace.c
    > @@ -7,6 +7,7 @@
    > #include <linux/module.h>
    > #include <linux/io.h>
    > #include <linux/mmiotrace.h>
    > +#include <linux/security.h>
    >
    > static unsigned long mmio_address;
    > module_param_hw(mmio_address, ulong, iomem, 0);
    > @@ -114,6 +115,10 @@ static void do_test_bulk_ioremapping(void)
    > static int __init init(void)
    > {
    > unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
    > + int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
    > +
    > + if (ret)
    > + return ret;
    >
    > if (mmio_address == 0) {
    > pr_err("you have to use the module argument mmio_address.\n");
    > diff --git a/include/linux/security.h b/include/linux/security.h
    > index 88064d7f6827..c649cb91e762 100644
    > --- a/include/linux/security.h
    > +++ b/include/linux/security.h
    > @@ -93,6 +93,7 @@ enum lockdown_reason {
    > LOCKDOWN_PCMCIA_CIS,
    > LOCKDOWN_TIOCSSERIAL,
    > LOCKDOWN_MODULE_PARAMETERS,
    > + LOCKDOWN_MMIOTRACE,
    > LOCKDOWN_INTEGRITY_MAX,
    > LOCKDOWN_CONFIDENTIALITY_MAX,
    > };
    > diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
    > index d03c4c296af7..cd86ed9f4d4b 100644
    > --- a/security/lockdown/lockdown.c
    > +++ b/security/lockdown/lockdown.c
    > @@ -29,6 +29,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
    > [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
    > [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
    > [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
    > + [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
    > [LOCKDOWN_INTEGRITY_MAX] = "integrity",
    > [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
    > };
    > --
    > 2.22.0.410.gd8fdbe21b5-goog
    >

    --
    Kees Cook

    \
     
     \ /
      Last update: 2019-06-23 02:04    [W:5.117 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site