On Mon, Apr 10, 2017 at 5:42 PM, Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
