[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEiveUeYB8jQoP9R9xSj+5dxc3cfCg46Z=C5TkfkYibDWrEbPQ@mail.gmail.com>
Date: Mon, 10 Apr 2017 20:27:55 +0200
From: Djalal Harouni <tixxdz@...il.com>
To: Casey Schaufler <casey@...aufler-ca.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Andy Lutomirski <luto@...nel.org>,
Kees Cook <keescook@...omium.org>, Andrew Morton <akpm@...ux-foundation.org>,
kernel-hardening@...ts.openwall.com,
LSM List <linux-security-module@...r.kernel.org>,
Linux API <linux-api@...r.kernel.org>, Dongsu Park <dpark@...teo.net>,
James Morris <james.l.morris@...cle.com>, "Serge E. Hallyn" <serge@...lyn.com>,
Paul Moore <paul@...l-moore.com>, Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH RFC v2 2/3] security: add the ModAutoRestrict Linux
Security Module
On Mon, Apr 10, 2017 at 5:42 PM, Casey Schaufler <casey@...aufler-ca.com> wrote:
> On 4/9/2017 3:42 AM, Djalal Harouni wrote:
[...]
>> +
>> +static inline struct modautoload_task *init_modautoload_task(struct task_struct *tsk,
>> + unsigned long flags)
>> +{
>> + struct modautoload_task *modtask;
>> +
>> + modtask = task_security(tsk, modautorestrict_task_security_index);
>> +
>> + modtask->flags = (u8)flags;
>
> I don't think you want to do this cast.
Will fix it. Thanks!
[...]
>> +
>> +#ifdef CONFIG_SYSCTL
>> +static int modautoload_dointvec_minmax(struct ctl_table *table, int write,
>> + void __user *buffer, size_t *lenp,
>> + loff_t *ppos)
>> +{
>> + struct ctl_table table_copy;
>> +
>> + if (write && !capable(CAP_SYS_MODULE))
>> + return -EPERM;
>> +
>> + table_copy = *table;
>> + if (*(int *)table_copy.data == *(int *)table_copy.extra2)
>
> While it's probably doing what you want, I find this
> sort of casting disturbing.
Ok will try to improve it.
>> + table_copy.extra1 = table_copy.extra2;
>> +
>> + return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
>> +}
>> +
>> +struct ctl_path modautoload_sysctl_path[] = {
>> + { .procname = "kernel", },
>> + { .procname = "modautorestrict", },
>> + { }
>> +};
>> +
>> +static struct ctl_table modautoload_sysctl_table[] = {
>> + {
>> + .procname = "autoload",
>> + .data = &autoload_restrict,
>> + .maxlen = sizeof(int),
>> + .mode = 0644,
>> + .proc_handler = modautoload_dointvec_minmax,
>> + .extra1 = &zero,
>> + .extra2 = &max_autoload_restrict,
>> + },
>> + { }
>> +};
>> +
>> +static void __init modautoload_init_sysctl(void)
>> +{
>> + if (!register_sysctl_paths(modautoload_sysctl_path, modautoload_sysctl_table))
>> + panic("modautorestrict: sysctl registration failed.\n");
>> +}
>> +#else
>> +static inline void modautoload_init_sysctl(void) { }
>> +#endif /* CONFIG_SYSCTL */
>> +
>> +void __init modautorestrict_init(void)
>> +{
>> + modautorestrict_task_security_index =
>> + security_reserve_task_blob_index(sizeof(struct modautoload_task));
>> + security_add_hooks(modautoload_hooks,
>> + ARRAY_SIZE(modautoload_hooks), "modautorestrict");
>> +
>> + modautoload_init_sysctl();
>> + pr_info("ModAutoRestrict LSM: Initialized\n");
>> +}
>> diff --git a/security/security.c b/security/security.c
>> index 4dc6bca..d8852fe 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -70,6 +70,7 @@ int __init security_init(void)
>> capability_add_hooks();
>> yama_add_hooks();
>> loadpin_add_hooks();
>> + modautorestrict_init();
>
> This should be modautorestrict_add_hooks() if this were
> a "minor" module, but as it's using a blob it is a "major"
> module. Either way, this is not right.
Do you mean that if I'm using a blob, it should go with the rest LSMs
in do_security_initcalls() ?
>>
>> /*
>> * Load all the remaining security modules.
>
Thanks for the comments!
--
tixxdz
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
