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
> >
>
