Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions

[Kees Cook]

On Mon, Mar 28, 2016 at 2:38 PM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 28 Mar 2016 14:14:22 -0700 Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
>> This LSM enforces that kernel-loaded files (modules, firmware, etc)
>> must all come from the same filesystem, with the expectation that
>> such a filesystem is backed by a read-only device such as dm-verity
>> or CDROM. This allows systems that have a verified and/or unchangeable
>> filesystem to enforce module and firmware loading restrictions without
>> needing to sign the files individually.
>
> Patchset generally looks good to me.  It's regrettable that a load of
> stuff was added to lib/ for one obscure LSM but hopefully (doubtfully)
> someone else will find a use for some of it.
>
> I'll assume that James is handling all of this.
>
>> --- /dev/null
>> +++ b/security/loadpin/loadpin.c
>> @@ -0,0 +1,206 @@
>> +/*
>> + * Module and Firmware Pinning Security Module
>> + *
>> + * Copyright 2011-2016 Google Inc.
>> + *
>> + * Author: Kees Cook <keescook@xxxxxxxxxxxx>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#define pr_fmt(fmt) "LoadPin: " fmt
>> +
>> +#include <linux/module.h>
>> +#include <linux/fs.h>
>> +#include <linux/fs_struct.h>
>> +#include <linux/lsm_hooks.h>
>> +#include <linux/mount.h>
>> +#include <linux/path.h>
>> +#include <linux/sched.h>     /* current */
>> +#include <linux/string_helpers.h>
>> +
>> +static void report_load(const char *origin, struct file *file, char *operation)
>> +{
>> +     char *cmdline, *pathname;
>> +
>> +     pathname = kstrdup_quotable_file(file);
>> +     cmdline = kstrdup_quotable_cmdline(current);
>> +
>> +     pr_notice("%s %s obj=%s%s%s pid=%d cmdline=%s%s%s\n",
>> +               origin, operation,
>> +               (pathname && pathname[0] != '<') ? "\"" : "",
>> +               pathname,
>> +               (pathname && pathname[0] != '<') ? "\"" : "",
>> +               task_pid_nr(current),
>> +               cmdline ? "\"" : "", cmdline, cmdline ? "\"" : "");
>> +
>> +     kfree(cmdline);
>> +     kfree(pathname);
>> +}
>> +
>> +static int load_pinning = 1;
>> +static struct super_block *pinned_root;
>> +static DEFINE_SPINLOCK(pinned_root_spinlock);
>> +
>> +#ifdef CONFIG_SYSCTL
>> +static int zero;
>> +static int one = 1;
>> +
>> +static struct ctl_path loadpin_sysctl_path[] = {
>> +     { .procname = "kernel", },
>> +     { }
>> +};
>> +
>> +static struct ctl_table loadpin_sysctl_table[] = {
>> +     {
>> +             .procname       = "load_pinning",
>> +             .data           = &load_pinning,
>> +             .maxlen         = sizeof(int),
>> +             .mode           = 0644,
>> +             .proc_handler   = proc_dointvec_minmax,
>> +             .extra1         = &zero,
>> +             .extra2         = &one,
>> +     },
>> +     { }
>> +};
>
> There should be somewhere to document the new sysctl?

Good call. I'll send a follow-up with a Documentation patch.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security