[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 24 Mar 2016 22:31:28 +0100
From: Mickaël Salaün <mic@...ikod.net>
To: Casey Schaufler <casey@...aufler-ca.com>,
linux-security-module@...r.kernel.org
Cc: Andreas Gruenbacher <agruenba@...hat.com>,
Andy Lutomirski <luto@...capital.net>, Andy Lutomirski <luto@...nel.org>,
Arnd Bergmann <arnd@...db.de>, Daniel Borkmann <daniel@...earbox.net>,
David Drysdale <drysdale@...gle.com>, Eric Paris <eparis@...hat.com>,
James Morris <james.l.morris@...cle.com>, Jeff Dike <jdike@...toit.com>,
Julien Tinnes <jln@...gle.com>, Kees Cook <keescook@...omium.org>,
Michael Kerrisk <mtk.manpages@...il.com>, Paul Moore <pmoore@...hat.com>,
Richard Weinberger <richard@....at>, "Serge E . Hallyn" <serge@...lyn.com>,
Stephen Smalley <sds@...ho.nsa.gov>,
Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
Will Drewry <wad@...omium.org>, linux-api@...r.kernel.org,
kernel-hardening@...ts.openwall.com
Subject: Re: [RFC v1 05/17] security/seccomp: Add LSM and create arrays of
syscall metadata
On 24/03/2016 17:01, Casey Schaufler wrote:
> On 3/23/2016 6:46 PM, Mickaël Salaün wrote:
>> diff --git a/security/seccomp/lsm.c b/security/seccomp/lsm.c
>> new file mode 100644
>> index 000000000000..93c881724341
>> --- /dev/null
>> +++ b/security/seccomp/lsm.c
>> @@ -0,0 +1,87 @@
>> +/*
>> + * Seccomp Linux Security Module
>> + *
>> + * Copyright (C) 2016 Mickaël Salaün <mic@...ikod.net>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2, as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <asm/syscall.h> /* sys_call_table */
>> +#include <linux/compat.h>
>> +#include <linux/slab.h> /* kcalloc() */
>> +#include <linux/syscalls.h> /* syscall_argdesc */
>> +
>> +#include "lsm.h"
>> +
>> +/* TODO: Remove the need for CONFIG_SYSFS dependency */
>> +
>> +struct syscall_argdesc (*seccomp_syscalls_argdesc)[] = NULL;
>> +#ifdef CONFIG_COMPAT
>> +struct syscall_argdesc (*compat_seccomp_syscalls_argdesc)[] = NULL;
>> +#endif /* CONFIG_COMPAT */
>> +
>> +static const struct syscall_argdesc *__init
>> +find_syscall_argdesc(const struct syscall_argdesc *start,
>> + const struct syscall_argdesc *stop, const void *addr)
>> +{
>> + if (unlikely(!addr || !start || !stop)) {
>> + WARN_ON(1);
>> + return NULL;
>> + }
>> +
>> + for (; start < stop; start++) {
>> + if (start->addr == addr)
>> + return start;
>> + }
>> + return NULL;
>> +}
>> +
>> +static inline void __init init_argdesc(void)
>> +{
>> + const struct syscall_argdesc *argdesc;
>> + const void *addr;
>> + int i;
>> +
>> + seccomp_syscalls_argdesc = kcalloc(NR_syscalls,
>> + sizeof((*seccomp_syscalls_argdesc)[0]), GFP_KERNEL);
>> + if (unlikely(!seccomp_syscalls_argdesc)) {
>> + WARN_ON(1);
>> + return;
>> + }
>> + for (i = 0; i < NR_syscalls; i++) {
>> + addr = sys_call_table[i];
>> + argdesc = find_syscall_argdesc(__start_syscalls_argdesc,
>> + __stop_syscalls_argdesc, addr);
>> + if (!argdesc)
>> + continue;
>> +
>> + (*seccomp_syscalls_argdesc)[i] = *argdesc;
>> + }
>> +
>> +#ifdef CONFIG_COMPAT
>> + compat_seccomp_syscalls_argdesc = kcalloc(IA32_NR_syscalls,
>> + sizeof((*compat_seccomp_syscalls_argdesc)[0]),
>> + GFP_KERNEL);
>> + if (unlikely(!compat_seccomp_syscalls_argdesc)) {
>> + WARN_ON(1);
>> + return;
>> + }
>> + for (i = 0; i < IA32_NR_syscalls; i++) {
>> + addr = ia32_sys_call_table[i];
>> + argdesc = find_syscall_argdesc(__start_compat_syscalls_argdesc,
>> + __stop_compat_syscalls_argdesc, addr);
>> + if (!argdesc)
>> + continue;
>> +
>> + (*compat_seccomp_syscalls_argdesc)[i] = *argdesc;
>> + }
>> +#endif /* CONFIG_COMPAT */
>> +}
>> +
>> +void __init seccomp_init(void)
>> +{
>> + pr_info("seccomp: Becoming ready for sandboxing\n");
>> + init_argdesc();
>> +}
>
> This isn't using the LSM infrastructure at all, is it?
> It looks like the only reason you're calling it a security
> module is to get the initialization code called in
> security_init().
>
> Let me amend my previous comment, which was to change
> the name of seccomp_init(). Leave it as is, but add a
> comment before it that explains why you've put the
> call in the midst of the security module initialization.
The patch "[RFC v1 16/17] security/seccomp: Protect against filesystem TOCTOU" add LSM hooks, so it make sense to follow your first comment and rename seccomp_init() to seccomp_add_hooks().
Mickaël
Download attachment "signature.asc" of type "application/pgp-signature" (456 bytes)
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.
