linux-kernel - Re: [PATCH v4 8/8] module: replace the existing LSM hook in init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1527635645.3534.39.camel@linux.vnet.ibm.com>
Date:   Tue, 29 May 2018 19:14:05 -0400
From:   Mimi Zohar <zohar@...ux.vnet.ibm.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, David Howells <dhowells@...hat.com>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>,
        Eric Biederman <ebiederm@...ssion.com>,
        kexec@...ts.infradead.org, Andres Rodriguez <andresx7@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Jeff Vander Stoep <jeffv@...gle.com>,
        Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v4 8/8] module: replace the existing LSM hook in
 init_module

On Tue, 2018-05-29 at 18:39 -0400, Paul Moore wrote:
[...]
> > @@ -4043,6 +4037,25 @@ static int selinux_kernel_module_from_file(struct file *file)
> >                                 SYSTEM__MODULE_LOAD, &ad);
> >  }
> >
> > +static int selinux_kernel_load_data(enum kernel_load_data_id id)
> > +{
> > +       u32 sid;
> > +       int rc = 0;
> > +
> > +       switch (id) {
> > +       case LOADING_MODULE:
> > +               sid = current_sid();
> > +
> > +               /* init_module */
> > +               return avc_has_perm(&selinux_state, sid, sid, SECCLASS_SYSTEM,
> > +                                   SYSTEM__MODULE_LOAD, NULL);
> > +       default:
> > +               break;
> > +       }
> > +
> > +       return rc;
> > +}
> 
> I'm not a fan of the duplication here.  If we must have a new LSM hook
> for this, can we at least have it call
> selinux_kernel_module_from_file() so we have all the kernel module
> loading logic/controls in one function?  Yes, I understand there are
> differences between init_module() and finit_module() but I like
> handling them both in one function as we do today.

There's some disagreement as to whether we really need two LSM hooks.
 This sounds like you would prefer a single LSM hook, not the two that
this patch set introduces.

We need to come to some consensus.  (Comments appreciated in 0/8.)

Mimi

> 
> >  static int selinux_kernel_read_file(struct file *file,
> >                                     enum kernel_read_file_id id)
> >  {
> > @@ -6950,6 +6963,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> >         LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
> >         LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
> >         LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
> > +       LSM_HOOK_INIT(kernel_load_data, selinux_kernel_load_data),
> >         LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file),
> >         LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
> >         LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> > --
> > 2.7.5
> >
>