Hi all,
Smackfs initialization without an enabled Smack leads to
an early Oops that renders the system unusable.
Introduce a global smack_enabled variable that will be used
to make sure that no smack components will be registered
(ala smackfs) if we are not already enabled.
Signed-off-by: Ahmed S. Darwish <[email protected]>
---
The Oops is triggered by the security= patch that will be sent
soon.
I can't imagine an SELinux guru finding /smackfs instead of
his usual /selinuxfs when he hits a tab completion after 's' ;).
As a bonus, this patch will handle that case too.
smack.h | 9 +++++++++
smack_lsm.c | 8 ++++++++
smackfs.c | 3 +++
3 files changed, 20 insertions(+)
diff --git a/security/smack/smack.h b/security/smack/smack.h
index a21a0e9..17c55ad 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -18,6 +18,15 @@
#include <net/netlabel.h>
/*
+ * We must not bother the rest of the kernel by exporting our
+ * own stuff if we are not already enabled. We may not be loaded
+ * if another or no LSM was chosen on boot.
+ * Smackfs is currently the only exported component, but this
+ * may change in the future.
+ */
+extern int smack_enabled;
+
+/*
* Why 23? CIPSO is constrained to 30, so a 32 byte buffer is
* bigger than can be used, and 24 is the next lower multiple
* of 8, and there are too many issues if there isn't space set
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 770eb06..6fe7869 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -36,6 +36,8 @@
#define SOCKFS_MAGIC 0x534F434B
#define TMPFS_MAGIC 0x01021994
+int smack_enabled;
+
/**
* smk_fetch - Fetch the smack label from a file.
* @ip: a pointer to the inode
@@ -2589,6 +2591,12 @@ static __init int smack_init(void)
if (register_security(&smack_ops))
panic("smack: Unable to register with kernel.\n");
+ /*
+ * Notify other Smack components that it's now safe to
+ * to register themselves.
+ */
+ smack_enabled = 1;
+
return 0;
}
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 358c92c..e1687c0 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -992,6 +992,9 @@ static int __init init_smk_fs(void)
{
int err;
+ if (!smack_enabled)
+ return 0;
+
err = register_filesystem(&smk_fs_type);
if (!err) {
smackfs_mount = kern_mount(&smk_fs_type);
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
Hi all,
[
Sending as a reply to the above Smack bugfix cause the bug
is triggered by this patch.
Please merge the bugfix before this.
IMHO this patch (or similar) is needed before the release
to avoid useless panics. Also as noted by Alexey, current
allyesconfig kernels panics cause of the same problem which
means less testing before hitting 2.6.25 .
Thanks!
]
-->
Add the security= boot parameter. This is done to avoid LSM
registration clashes in case of more than one bult-in module.
User can choose a security module to enable at boot. If no
security= boot parameter is specified, only the first LSM
asking for registration will be loaded. An invalid security
module name will be treated as if no module has been chosen.
LSM modules must check now if they are allowed to register
by calling security_module_enable(ops) first. Modify SELinux
and SMACK to do so.
Signed-off-by: Ahmed S. Darwish <[email protected]>
Acked-by: James Morris <[email protected]>
Reported-by: Alexey Dobriyan <[email protected]>
---
[James ACKed the -mm version, but this patch is exactly the same]
Documentation/kernel-parameters.txt | 6 ++++
include/linux/security.h | 12 +++++++++
security/dummy.c | 2 -
security/security.c | 46 +++++++++++++++++++++++++++++++++++-
security/selinux/hooks.c | 7 +++++
security/smack/smack_lsm.c | 5 +++
6 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9a5b665..64efbdc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -374,6 +374,12 @@ and is between 256 and 4096 characters. It is defined in the file
possible to determine what the correct size should be.
This option provides an override for these situations.
+ security= [SECURITY] Choose a security module to enable at boot.
+ If this boot parameter is not specified, only the first
+ security module asking for security registration will be
+ loaded. An invalid security module name will be treated
+ as if no module has been chosen.
+
capability.disable=
[SECURITY] Disable capabilities. This would normally
be used only if an alternative security model is to be
diff --git a/include/linux/security.h b/include/linux/security.h
index fe52cde..801c9ad 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -42,6 +42,9 @@
extern unsigned securebits;
+/* Maximum number of letters for an LSM name string */
+#define SECURITY_NAME_MAX 10
+
struct ctl_table;
/*
@@ -117,6 +120,12 @@ struct request_sock;
/**
* struct security_operations - main security structure
*
+ * Security module identifier.
+ *
+ * @name:
+ * A string that acts as a unique identifeir for the LSM with max number
+ * of characters = SECURITY_NAME_MAX.
+ *
* Security hooks for program execution operations.
*
* @bprm_alloc_security:
@@ -1207,6 +1216,8 @@ struct request_sock;
* This is the main security structure.
*/
struct security_operations {
+ char name[SECURITY_NAME_MAX + 1];
+
int (*ptrace) (struct task_struct * parent, struct task_struct * child);
int (*capget) (struct task_struct * target,
kernel_cap_t * effective,
@@ -1466,6 +1477,7 @@ struct security_operations {
/* prototypes */
extern int security_init (void);
+extern int security_module_enable(struct security_operations *ops);
extern int register_security (struct security_operations *ops);
extern int mod_reg_security (const char *name, struct security_operations *ops);
extern struct dentry *securityfs_create_file(const char *name, mode_t mode,
diff --git a/security/dummy.c b/security/dummy.c
index 649326b..96d196f 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -979,7 +979,7 @@ static inline int dummy_key_permission(key_ref_t key_ref,
}
#endif /* CONFIG_KEYS */
-struct security_operations dummy_security_ops;
+struct security_operations dummy_security_ops = { "dummy" };
#define set_to_dummy_if_null(ops, function) \
do { \
diff --git a/security/security.c b/security/security.c
index d15e56c..f188672 100644
--- a/security/security.c
+++ b/security/security.c
@@ -17,6 +17,9 @@
#include <linux/kernel.h>
#include <linux/security.h>
+/* Boot-time LSM user choice */
+static spinlock_t chosen_lsm_lock;
+static char chosen_lsm[SECURITY_NAME_MAX + 1];
/* things that live in dummy.c */
extern struct security_operations dummy_security_ops;
@@ -62,18 +65,59 @@ int __init security_init(void)
}
security_ops = &dummy_security_ops;
+ spin_lock_init(&chosen_lsm_lock);
do_security_initcalls();
return 0;
}
+/* Save user chosen LSM */
+static int __init choose_lsm(char *str)
+{
+ strncpy(chosen_lsm, str, SECURITY_NAME_MAX);
+ return 1;
+}
+__setup("security=", choose_lsm);
+
+/**
+ * security_module_enable - Load given security module on boot ?
+ * @ops: a pointer to the struct security_operations that is to be checked.
+ *
+ * Each LSM must pass this method before registering its own operations
+ * to avoid security registration races.
+ *
+ * Return true if:
+ * -The passed LSM is the one chosen by user at boot time,
+ * -or user didsn't specify a specific LSM and we're the first to ask
+ * for registeration permissoin.
+ * Otherwise, return false.
+ */
+int security_module_enable(struct security_operations *ops)
+{
+ int rc = 1;
+
+ spin_lock(&chosen_lsm_lock);
+ if (!*chosen_lsm)
+ strncpy(chosen_lsm, ops->name, SECURITY_NAME_MAX);
+ else if (strncmp(ops->name, chosen_lsm, SECURITY_NAME_MAX))
+ rc = 0;
+ spin_unlock(&chosen_lsm_lock);
+
+ if (rc)
+ printk(KERN_INFO "Security: Loading '%s' security module.\n",
+ ops->name);
+
+ return rc;
+}
+
/**
* register_security - registers a security framework with the kernel
* @ops: a pointer to the struct security_options that is to be registered
*
* This function is to allow a security module to register itself with the
* kernel security subsystem. Some rudimentary checking is done on the @ops
- * value passed to this function.
+ * value passed to this function. You'll need to check first if your LSM
+ * is allowed to register its @ops by calling security_module_enable(@ops).
*
* If there is already a security module registered with the kernel,
* an error will be returned. Otherwise 0 is returned on success.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 75c2e99..49709a4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5219,6 +5219,8 @@ static int selinux_key_permission(key_ref_t key_ref,
#endif
static struct security_operations selinux_ops = {
+ .name = "selinux",
+
.ptrace = selinux_ptrace,
.capget = selinux_capget,
.capset_check = selinux_capset_check,
@@ -5405,6 +5407,11 @@ static __init int selinux_init(void)
{
struct task_security_struct *tsec;
+ if (!security_module_enable(&selinux_ops)) {
+ selinux_enabled = 0;
+ return 0;
+ }
+
if (!selinux_enabled) {
printk(KERN_INFO "SELinux: Disabled at boot.\n");
return 0;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 6fe7869..ecf1d16 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2436,6 +2436,8 @@ static void smack_release_secctx(char *secdata, u32 seclen)
}
static struct security_operations smack_ops = {
+ .name = "smack",
+
.ptrace = smack_ptrace,
.capget = cap_capget,
.capset_check = cap_capset_check,
@@ -2568,6 +2570,9 @@ static struct security_operations smack_ops = {
*/
static __init int smack_init(void)
{
+ if (!security_module_enable(&smack_ops))
+ return 0;
+
printk(KERN_INFO "Smack: Initializing.\n");
/*
If you read till here, Thank you for the review ;)
Regards,
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
----- Original Message ----
> From: Ahmed S. Darwish <[email protected]>
> To: Casey Schaufler <[email protected]>
> Cc: LKML <[email protected]>; Linus <[email protected]>
> Sent: Tuesday, March 4, 2008 5:10:55 AM
> Subject: [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
>
> Hi all,
>
> Smackfs initialization without an enabled Smack leads to
> an early Oops that renders the system unusable.
>
> Introduce a global smack_enabled variable that will be used
> to make sure that no smack components will be registered
> (ala smackfs) if we are not already enabled.
>
> Signed-off-by: Ahmed S. Darwish
Acked-by: Casey Schaufler [email protected]
> ---
>
> The Oops is triggered by the security= patch that will be sent
> soon.
>
> I can't imagine an SELinux guru finding /smackfs instead of
> his usual /selinuxfs when he hits a tab completion after 's' ;).
> As a bonus, this patch will handle that case too.
>
> smack.h | 9 +++++++++
> smack_lsm.c | 8 ++++++++
> smackfs.c | 3 +++
>
> 3 files changed, 20 insertions(+)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index a21a0e9..17c55ad 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -18,6 +18,15 @@
> #include
>
> /*
> + * We must not bother the rest of the kernel by exporting our
> + * own stuff if we are not already enabled. We may not be loaded
> + * if another or no LSM was chosen on boot.
> + * Smackfs is currently the only exported component, but this
> + * may change in the future.
> + */
> +extern int smack_enabled;
> +
> +/*
> * Why 23? CIPSO is constrained to 30, so a 32 byte buffer is
> * bigger than can be used, and 24 is the next lower multiple
> * of 8, and there are too many issues if there isn't space set
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 770eb06..6fe7869 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -36,6 +36,8 @@
> #define SOCKFS_MAGIC 0x534F434B
> #define TMPFS_MAGIC 0x01021994
>
> +int smack_enabled;
> +
> /**
> * smk_fetch - Fetch the smack label from a file.
> * @ip: a pointer to the inode
> @@ -2589,6 +2591,12 @@ static __init int smack_init(void)
> if (register_security(&smack_ops))
> panic("smack: Unable to register with kernel.\n");
>
> + /*
> + * Notify other Smack components that it's now safe to
> + * to register themselves.
> + */
> + smack_enabled = 1;
> +
> return 0;
> }
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 358c92c..e1687c0 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -992,6 +992,9 @@ static int __init init_smk_fs(void)
> {
> int err;
>
> + if (!smack_enabled)
> + return 0;
> +
> err = register_filesystem(&smk_fs_type);
> if (!err) {
> smackfs_mount = kern_mount(&smk_fs_type);
>
> --
>
> "Better to light a candle, than curse the darkness"
>
> Ahmed S. Darwish
> Homepage: http://darwish.07.googlepages.com
> Blog: http://darwish-07.blogspot.com
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at
> http://www.tux.org/lkml/
Casey Schaufler
[email protected]
On Tue, 4 Mar 2008, Ahmed S. Darwish wrote:
>
> Smackfs initialization without an enabled Smack leads to
> an early Oops that renders the system unusable.
I really think this is bogus. Global enables like this are just wrong, and
a sign that something else bad is going on.
What is the oops? Why does it happen?
Linus
----- Original Message ----
> From: Linus Torvalds <[email protected]>
> To: Ahmed S. Darwish <[email protected]>
> Cc: Casey Schaufler <[email protected]>; LKML <[email protected]>
> Sent: Tuesday, March 4, 2008 9:21:19 AM
> Subject: Re: [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
>
>
>
> On Tue, 4 Mar 2008, Ahmed S. Darwish wrote:
> >
> > Smackfs initialization without an enabled Smack leads to
> > an early Oops that renders the system unusable.
>
> I really think this is bogus. Global enables like this are just wrong, and
> a sign that something else bad is going on.
>
> What is the oops? Why does it happen?
A kernel that is built with both SELinux and Smack contains
all of the components for both, including smackfs. If SELinux
is chosen as the module to be used and smackfs is initialized
the oops occurs because the Smack initialization that smackfs
depends on has not been done.
One solution would be to tighten the smackfs code so that it
handles the uninitialized LSM case properly.
Another would be to set up Kconfig as to make SELinux and Smack
mutually exclusive, although I really don't know how well that
would go over in testing circles because "config all on" becomes
ambiguous.
A third would be to provide for stacking, but I assume that's
beyond the scope of this exercise.
So I think that fixing up smackfs is the right choice at this point.
>
>
> Linus
Casey Schaufler
[email protected]
On Tue, 4 Mar 2008, Casey Schaufler wrote:
>
> One solution would be to tighten the smackfs code so that it
> handles the uninitialized LSM case properly.
I really would tend to prefer that.
We had a very similar situation in the ACPI/WMI code, where some WMI
helper routines would crash just because the WMI data structures hadn't
been initialized.
The proper fix in that case (in my opinion, and the one that thus got
committed ;^) was simply to make sure that the data structures were simply
always consistent (and empty), even if the code itself was disabled. That
just automatically meant that it didn't need any global flags to be
tested.
So _if_ the alternative is as simple as just making sure that smackfs
keeps its data structures consistent even when disabled, I think that's
the right patch, rather than adding a separate hack to not touch them.
Linus
Hi Linus,
[Adding SELinux devs to CC list, please follow to the SELinux point.]
On Tue, Mar 04, 2008 at 09:21:19AM -0800, Linus Torvalds wrote:
>
>
> On Tue, 4 Mar 2008, Ahmed S. Darwish wrote:
> >
> > Smackfs initialization without an enabled Smack leads to
> > an early Oops that renders the system unusable.
>
> I really think this is bogus. Global enables like this are just wrong, and
> a sign that something else bad is going on.
>
> What is the oops? Why does it happen?
>
The problem occurs when Smack is built-in the kernel but not chosen
to register itself on boot. Smack was not chosen on boot cause either
security=AnotherLSM or security=NonExistentLSM.
In all cases, init_smk_fs() ,which registers smackfs, got called
cause it's an __initcall(init_smack_fs).
This include the cases where smack __was not__ chosen on boot.
Making smackfs mountable when Smack is not registered leads to:
1- an Oops by dereferncing the NULL security pointer: current->security (*)
2- Smackfs code got executed though naturally all the code assumes
that smack is already registered with the security system leading
to several problems.
3- The bogus idea of having a subsystem interface available when the
subsystem itself is not available!
So the global is used in init_smk_fs to not register smackfs if
Smack wasn't enabled on boot.
---- SELinux:
I think the SELinux folks faced the same problem too. In my first
local iteration of the security= parameter patch, I forgot to set
`selinux_disable = 1' if SELinux wasn't chosen on boot.
This led to dozen of SELinux Udev events and also led to selinuxfs
being available even though SELinux hooks _weren't_ registered.
Regards,
(*)
Could not save the oops cause it occured too early, but
it was like this:
__init_call
init_smk_fs(void)
smk_unlbl_ambient(NULL)
/*
* Here: current->security = NULL, cause SMACK initial setup
* was not executed.
*/
smack_to_secid(current->security)
strncmp(.., current->security, ..)
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
On Tue, 4 Mar 2008, Linus Torvalds wrote:
>
>
> On Tue, 4 Mar 2008, Casey Schaufler wrote:
> >
> > One solution would be to tighten the smackfs code so that it
> > handles the uninitialized LSM case properly.
>
> I really would tend to prefer that.
Can you simply call init_smk_fs() from smack_init() prior to
register_security() ?
- James
--
James Morris
<[email protected]>
On Wed, Mar 05, 2008 at 11:58:08AM +1100, James Morris wrote:
> On Tue, 4 Mar 2008, Linus Torvalds wrote:
>
> >
> >
> > On Tue, 4 Mar 2008, Casey Schaufler wrote:
> > >
> > > One solution would be to tighten the smackfs code so that it
> > > handles the uninitialized LSM case properly.
> >
> > I really would tend to prefer that.
>
> Can you simply call init_smk_fs() from smack_init() prior to
> register_security() ?
>
After analysing below oops:
[ 0.072989] Call Trace:
[ 0.072989] [<c017d4a4>] alloc_vfsmnt+0x24/0xd0
[ 0.072989] [<c0168ee1>] vfs_kern_mount+0x31/0x120
[ 0.072989] [<c0168fe2>] kern_mount_data+0x12/0x20
[ 0.072989] [<c038a73c>] init_smk_fs+0x4c/0x80
[ 0.072989] [<c038a6ce>] smack_init+0xae/0xd0
[ 0.072989] [<c038a192>] security_init+0x52/0x70
[ 0.072989] [<c037aaa5>] start_kernel+0x1e5/0x290
[ 0.072989] [<c037a380>] unknown_bootoption+0x0/0x1f0
[ 0.072989] =======================
I've found that vfs_caches_init() got called after calling
security_init() in the main kernel init path.
This leads to kern_mount() Oopsing cause of Null dereferencing
of mnt_cache in the alloc_vfsmnt(mnt_cache, GFP_KERNEL) step.
I was thinking about exporting the current chosen_lsm[] string,
but I don't know if this maybe seen by Linus as another global
that arises from bad design. It may not also scale well if the
LSM needs to check if it's enabled in various places (lots of
strcmp()s).
Regards,
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
On Tue, Mar 04, 2008 at 09:45:04AM -0800, Casey Schaufler wrote:
> ----- Original Message ----
> > From: Linus Torvalds <[email protected]>
> > To: Ahmed S. Darwish <[email protected]>
> > Cc: Casey Schaufler <[email protected]>; LKML <[email protected]>
> > Sent: Tuesday, March 4, 2008 9:21:19 AM
> > Subject: Re: [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
> >
> >
> >
> > On Tue, 4 Mar 2008, Ahmed S. Darwish wrote:
> > >
> > > Smackfs initialization without an enabled Smack leads to
> > > an early Oops that renders the system unusable.
> >
> > I really think this is bogus. Global enables like this are just wrong, and
> > a sign that something else bad is going on.
> >
> > What is the oops? Why does it happen?
...
>
> One solution would be to tighten the smackfs code so that it
> handles the uninitialized LSM case properly.
>
IMHO no smackfs code should ever execute if smack isn't loaded.
This means catching it from the very fist step where it registers
itself in init_smk_fs instead of doing several if(we're enabled) cases
in the code path.
The solution should be a _general_ solution, _not_ a SMACK one cause
SELinux sufferes from exactly the same problem.
a.k.a:
LSMs need a scalable way to know if they're enabled that makes
everyone happy ( especially Linus ;) ).
Regads to all,
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
On Wed, Mar 05, 2008 at 02:44:45PM +0200, Ahmed S. Darwish wrote:
> On Tue, Mar 04, 2008 at 09:45:04AM -0800, Casey Schaufler wrote:
> > > From: Linus Torvalds <[email protected]>
...
> > >
> > > What is the oops? Why does it happen?
> ...
> >
> > One solution would be to tighten the smackfs code so that it
> > handles the uninitialized LSM case properly.
> >
>
> IMHO no smackfs code should ever execute if smack isn't loaded.
>
> This means catching it from the very fist step where it registers
> itself in init_smk_fs instead of doing several if(we're enabled) cases
> in the code path.
>
> The solution should be a _general_ solution, _not_ a SMACK one cause
> SELinux sufferes from exactly the same problem.
>
Not to be misunderstood here, I'm used to writing SMACK in capitals.
I ,ofcourse, don't mean shouting (never will).
Regards,
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
Hi!,
[
Our usual changelog:
- Fold the SMACK bugfix here cause now it depends on the
new introduced security_ symbol.
- Do not register smackfs if Smack was not loaded
- Do not use a global to check if Smack was enabled, use
security_module_enable(ops) instead.
- Export smack_ops security operations to the rest of
SMACK code to satisfy above point.
- Remove James ACK cause patch semantics changed (could you
please reACK ?).
]
-->
Add the security= boot parameter. This is done to avoid LSM
registration clashes in case of more than one bult-in module.
User can choose a security module to enable at boot. If no
security= boot parameter is specified, only the first LSM
asking for registration will be loaded. An invalid security
module name will be treated as if no module has been chosen.
LSM modules must check now if they are allowed to register
by calling security_module_enable(ops) first. Modify SELinux
and SMACK to do so.
Do not let SMACK register smackfs if it was not chosen on
boot. Smackfs assumes that smack hooks are registered and
the initial task security setup (swapper->security) is done.
Signed-off-by: Ahmed S. Darwish <[email protected]>
Reported-by: Alexey Dobriyan <[email protected]>
---
Documentation/kernel-parameters.txt | 6 ++++
include/linux/security.h | 12 +++++++++
security/dummy.c | 2 -
security/security.c | 46 +++++++++++++++++++++++++++++++++++-
security/selinux/hooks.c | 7 +++++
security/smack/smack.h | 2 +
security/smack/smack_lsm.c | 7 ++++-
security/smack/smackfs.c | 9 ++++++-
8 files changed, 87 insertions(+), 4 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9a5b665..64efbdc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -374,6 +374,12 @@ and is between 256 and 4096 characters. It is defined in the file
possible to determine what the correct size should be.
This option provides an override for these situations.
+ security= [SECURITY] Choose a security module to enable at boot.
+ If this boot parameter is not specified, only the first
+ security module asking for security registration will be
+ loaded. An invalid security module name will be treated
+ as if no module has been chosen.
+
capability.disable=
[SECURITY] Disable capabilities. This would normally
be used only if an alternative security model is to be
diff --git a/include/linux/security.h b/include/linux/security.h
index fe52cde..801c9ad 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -42,6 +42,9 @@
extern unsigned securebits;
+/* Maximum number of letters for an LSM name string */
+#define SECURITY_NAME_MAX 10
+
struct ctl_table;
/*
@@ -117,6 +120,12 @@ struct request_sock;
/**
* struct security_operations - main security structure
*
+ * Security module identifier.
+ *
+ * @name:
+ * A string that acts as a unique identifeir for the LSM with max number
+ * of characters = SECURITY_NAME_MAX.
+ *
* Security hooks for program execution operations.
*
* @bprm_alloc_security:
@@ -1207,6 +1216,8 @@ struct request_sock;
* This is the main security structure.
*/
struct security_operations {
+ char name[SECURITY_NAME_MAX + 1];
+
int (*ptrace) (struct task_struct * parent, struct task_struct * child);
int (*capget) (struct task_struct * target,
kernel_cap_t * effective,
@@ -1466,6 +1477,7 @@ struct security_operations {
/* prototypes */
extern int security_init (void);
+extern int security_module_enable(struct security_operations *ops);
extern int register_security (struct security_operations *ops);
extern int mod_reg_security (const char *name, struct security_operations *ops);
extern struct dentry *securityfs_create_file(const char *name, mode_t mode,
diff --git a/security/dummy.c b/security/dummy.c
index 649326b..96d196f 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -979,7 +979,7 @@ static inline int dummy_key_permission(key_ref_t key_ref,
}
#endif /* CONFIG_KEYS */
-struct security_operations dummy_security_ops;
+struct security_operations dummy_security_ops = { "dummy" };
#define set_to_dummy_if_null(ops, function) \
do { \
diff --git a/security/security.c b/security/security.c
index d15e56c..f188672 100644
--- a/security/security.c
+++ b/security/security.c
@@ -17,6 +17,9 @@
#include <linux/kernel.h>
#include <linux/security.h>
+/* Boot-time LSM user choice */
+static spinlock_t chosen_lsm_lock;
+static char chosen_lsm[SECURITY_NAME_MAX + 1];
/* things that live in dummy.c */
extern struct security_operations dummy_security_ops;
@@ -62,18 +65,59 @@ int __init security_init(void)
}
security_ops = &dummy_security_ops;
+ spin_lock_init(&chosen_lsm_lock);
do_security_initcalls();
return 0;
}
+/* Save user chosen LSM */
+static int __init choose_lsm(char *str)
+{
+ strncpy(chosen_lsm, str, SECURITY_NAME_MAX);
+ return 1;
+}
+__setup("security=", choose_lsm);
+
+/**
+ * security_module_enable - Load given security module on boot ?
+ * @ops: a pointer to the struct security_operations that is to be checked.
+ *
+ * Each LSM must pass this method before registering its own operations
+ * to avoid security registration races.
+ *
+ * Return true if:
+ * -The passed LSM is the one chosen by user at boot time,
+ * -or user didsn't specify a specific LSM and we're the first to ask
+ * for registeration permissoin.
+ * Otherwise, return false.
+ */
+int security_module_enable(struct security_operations *ops)
+{
+ int rc = 1;
+
+ spin_lock(&chosen_lsm_lock);
+ if (!*chosen_lsm)
+ strncpy(chosen_lsm, ops->name, SECURITY_NAME_MAX);
+ else if (strncmp(ops->name, chosen_lsm, SECURITY_NAME_MAX))
+ rc = 0;
+ spin_unlock(&chosen_lsm_lock);
+
+ if (rc)
+ printk(KERN_INFO "Security: Loading '%s' security module.\n",
+ ops->name);
+
+ return rc;
+}
+
/**
* register_security - registers a security framework with the kernel
* @ops: a pointer to the struct security_options that is to be registered
*
* This function is to allow a security module to register itself with the
* kernel security subsystem. Some rudimentary checking is done on the @ops
- * value passed to this function.
+ * value passed to this function. You'll need to check first if your LSM
+ * is allowed to register its @ops by calling security_module_enable(@ops).
*
* If there is already a security module registered with the kernel,
* an error will be returned. Otherwise 0 is returned on success.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 75c2e99..49709a4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5219,6 +5219,8 @@ static int selinux_key_permission(key_ref_t key_ref,
#endif
static struct security_operations selinux_ops = {
+ .name = "selinux",
+
.ptrace = selinux_ptrace,
.capget = selinux_capget,
.capset_check = selinux_capset_check,
@@ -5405,6 +5407,11 @@ static __init int selinux_init(void)
{
struct task_security_struct *tsec;
+ if (!security_module_enable(&selinux_ops)) {
+ selinux_enabled = 0;
+ return 0;
+ }
+
if (!selinux_enabled) {
printk(KERN_INFO "SELinux: Disabled at boot.\n");
return 0;
diff --git a/security/smack/smack.h b/security/smack/smack.h
index a21a0e9..c444f48 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -15,6 +15,7 @@
#include <linux/capability.h>
#include <linux/spinlock.h>
+#include <linux/security.h>
#include <net/netlabel.h>
/*
@@ -195,6 +196,7 @@ extern struct smack_known smack_known_star;
extern struct smack_known smack_known_unset;
extern struct smk_list_entry *smack_list;
+extern struct security_operations smack_ops;
/*
* Stricly for CIPSO level manipulation.
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 770eb06..afa7967 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2433,7 +2433,9 @@ static void smack_release_secctx(char *secdata, u32 seclen)
{
}
-static struct security_operations smack_ops = {
+struct security_operations smack_ops = {
+ .name = "smack",
+
.ptrace = smack_ptrace,
.capget = cap_capget,
.capset_check = cap_capset_check,
@@ -2566,6 +2568,9 @@ static struct security_operations smack_ops = {
*/
static __init int smack_init(void)
{
+ if (!security_module_enable(&smack_ops))
+ return 0;
+
printk(KERN_INFO "Smack: Initializing.\n");
/*
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 358c92c..769da9a 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -986,12 +986,19 @@ static struct vfsmount *smackfs_mount;
*
* register the smackfs
*
- * Returns 0 unless the registration fails.
+ * Do not register smackfs if Smack wasn't enabled
+ * on boot.
+ *
+ * Returns true if we were not chosen on boot or if
+ * we were chosen and filesystem registration succeeded.
*/
static int __init init_smk_fs(void)
{
int err;
+ if (!security_module_enable(&smack_ops))
+ return 0;
+
err = register_filesystem(&smk_fs_type);
if (!err) {
smackfs_mount = kern_mount(&smk_fs_type);
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
--- "Ahmed S. Darwish" <[email protected]> wrote:
> Hi!,
>
> [
> Our usual changelog:
>
> - Fold the SMACK bugfix here cause now it depends on the
> new introduced security_ symbol.
> - Do not register smackfs if Smack was not loaded
> - Do not use a global to check if Smack was enabled, use
> security_module_enable(ops) instead.
> - Export smack_ops security operations to the rest of
> SMACK code to satisfy above point.
> - Remove James ACK cause patch semantics changed (could you
> please reACK ?).
> ]
>
> -->
>
> Add the security= boot parameter. This is done to avoid LSM
> registration clashes in case of more than one bult-in module.
>
> User can choose a security module to enable at boot. If no
> security= boot parameter is specified, only the first LSM
> asking for registration will be loaded. An invalid security
> module name will be treated as if no module has been chosen.
>
> LSM modules must check now if they are allowed to register
> by calling security_module_enable(ops) first. Modify SELinux
> and SMACK to do so.
>
> Do not let SMACK register smackfs if it was not chosen on
> boot. Smackfs assumes that smack hooks are registered and
> the initial task security setup (swapper->security) is done.
If the problem with initializing smackfs is because the
locks aren't initialized why not leave the lock initializations
in smack_init, and have them done before the check to see if the
smack LSM is going to get used? Really, we're only talking
about the case where a kernel is configured for testing or
development purposes, and the lock initialization can't
be considered a major impact in any case.
> Signed-off-by: Ahmed S. Darwish <[email protected]>
> Reported-by: Alexey Dobriyan <[email protected]>
> ---
>
> Documentation/kernel-parameters.txt | 6 ++++
> include/linux/security.h | 12 +++++++++
> security/dummy.c | 2 -
> security/security.c | 46
> +++++++++++++++++++++++++++++++++++-
> security/selinux/hooks.c | 7 +++++
> security/smack/smack.h | 2 +
> security/smack/smack_lsm.c | 7 ++++-
> security/smack/smackfs.c | 9 ++++++-
>
> 8 files changed, 87 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> index 9a5b665..64efbdc 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -374,6 +374,12 @@ and is between 256 and 4096 characters. It is defined in
> the file
> possible to determine what the correct size should be.
> This option provides an override for these situations.
>
> + security= [SECURITY] Choose a security module to enable at boot.
> + If this boot parameter is not specified, only the first
> + security module asking for security registration will be
> + loaded. An invalid security module name will be treated
> + as if no module has been chosen.
> +
> capability.disable=
> [SECURITY] Disable capabilities. This would normally
> be used only if an alternative security model is to be
> diff --git a/include/linux/security.h b/include/linux/security.h
> index fe52cde..801c9ad 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -42,6 +42,9 @@
>
> extern unsigned securebits;
>
> +/* Maximum number of letters for an LSM name string */
> +#define SECURITY_NAME_MAX 10
> +
> struct ctl_table;
>
> /*
> @@ -117,6 +120,12 @@ struct request_sock;
> /**
> * struct security_operations - main security structure
> *
> + * Security module identifier.
> + *
> + * @name:
> + * A string that acts as a unique identifeir for the LSM with max number
> + * of characters = SECURITY_NAME_MAX.
> + *
> * Security hooks for program execution operations.
> *
> * @bprm_alloc_security:
> @@ -1207,6 +1216,8 @@ struct request_sock;
> * This is the main security structure.
> */
> struct security_operations {
> + char name[SECURITY_NAME_MAX + 1];
> +
> int (*ptrace) (struct task_struct * parent, struct task_struct * child);
> int (*capget) (struct task_struct * target,
> kernel_cap_t * effective,
> @@ -1466,6 +1477,7 @@ struct security_operations {
>
> /* prototypes */
> extern int security_init (void);
> +extern int security_module_enable(struct security_operations *ops);
> extern int register_security (struct security_operations *ops);
> extern int mod_reg_security (const char *name, struct security_operations
> *ops);
> extern struct dentry *securityfs_create_file(const char *name, mode_t mode,
> diff --git a/security/dummy.c b/security/dummy.c
> index 649326b..96d196f 100644
> --- a/security/dummy.c
> +++ b/security/dummy.c
> @@ -979,7 +979,7 @@ static inline int dummy_key_permission(key_ref_t key_ref,
> }
> #endif /* CONFIG_KEYS */
>
> -struct security_operations dummy_security_ops;
> +struct security_operations dummy_security_ops = { "dummy" };
>
> #define set_to_dummy_if_null(ops, function) \
> do { \
> diff --git a/security/security.c b/security/security.c
> index d15e56c..f188672 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -17,6 +17,9 @@
> #include <linux/kernel.h>
> #include <linux/security.h>
>
> +/* Boot-time LSM user choice */
> +static spinlock_t chosen_lsm_lock;
> +static char chosen_lsm[SECURITY_NAME_MAX + 1];
>
> /* things that live in dummy.c */
> extern struct security_operations dummy_security_ops;
> @@ -62,18 +65,59 @@ int __init security_init(void)
> }
>
> security_ops = &dummy_security_ops;
> + spin_lock_init(&chosen_lsm_lock);
> do_security_initcalls();
>
> return 0;
> }
>
> +/* Save user chosen LSM */
> +static int __init choose_lsm(char *str)
> +{
> + strncpy(chosen_lsm, str, SECURITY_NAME_MAX);
> + return 1;
> +}
> +__setup("security=", choose_lsm);
> +
> +/**
> + * security_module_enable - Load given security module on boot ?
> + * @ops: a pointer to the struct security_operations that is to be checked.
> + *
> + * Each LSM must pass this method before registering its own operations
> + * to avoid security registration races.
> + *
> + * Return true if:
> + * -The passed LSM is the one chosen by user at boot time,
> + * -or user didsn't specify a specific LSM and we're the first to ask
> + * for registeration permissoin.
> + * Otherwise, return false.
> + */
> +int security_module_enable(struct security_operations *ops)
> +{
> + int rc = 1;
> +
> + spin_lock(&chosen_lsm_lock);
> + if (!*chosen_lsm)
> + strncpy(chosen_lsm, ops->name, SECURITY_NAME_MAX);
> + else if (strncmp(ops->name, chosen_lsm, SECURITY_NAME_MAX))
> + rc = 0;
> + spin_unlock(&chosen_lsm_lock);
> +
> + if (rc)
> + printk(KERN_INFO "Security: Loading '%s' security module.\n",
> + ops->name);
> +
> + return rc;
> +}
> +
> /**
> * register_security - registers a security framework with the kernel
> * @ops: a pointer to the struct security_options that is to be registered
> *
> * This function is to allow a security module to register itself with the
> * kernel security subsystem. Some rudimentary checking is done on the @ops
> - * value passed to this function.
> + * value passed to this function. You'll need to check first if your LSM
> + * is allowed to register its @ops by calling security_module_enable(@ops).
> *
> * If there is already a security module registered with the kernel,
> * an error will be returned. Otherwise 0 is returned on success.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 75c2e99..49709a4 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5219,6 +5219,8 @@ static int selinux_key_permission(key_ref_t key_ref,
> #endif
>
> static struct security_operations selinux_ops = {
> + .name = "selinux",
> +
> .ptrace = selinux_ptrace,
> .capget = selinux_capget,
> .capset_check = selinux_capset_check,
> @@ -5405,6 +5407,11 @@ static __init int selinux_init(void)
> {
> struct task_security_struct *tsec;
>
> + if (!security_module_enable(&selinux_ops)) {
> + selinux_enabled = 0;
> + return 0;
> + }
> +
> if (!selinux_enabled) {
> printk(KERN_INFO "SELinux: Disabled at boot.\n");
> return 0;
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index a21a0e9..c444f48 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -15,6 +15,7 @@
>
> #include <linux/capability.h>
> #include <linux/spinlock.h>
> +#include <linux/security.h>
> #include <net/netlabel.h>
>
> /*
> @@ -195,6 +196,7 @@ extern struct smack_known smack_known_star;
> extern struct smack_known smack_known_unset;
>
> extern struct smk_list_entry *smack_list;
> +extern struct security_operations smack_ops;
>
> /*
> * Stricly for CIPSO level manipulation.
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 770eb06..afa7967 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2433,7 +2433,9 @@ static void smack_release_secctx(char *secdata, u32
> seclen)
> {
> }
>
> -static struct security_operations smack_ops = {
> +struct security_operations smack_ops = {
> + .name = "smack",
> +
> .ptrace = smack_ptrace,
> .capget = cap_capget,
> .capset_check = cap_capset_check,
> @@ -2566,6 +2568,9 @@ static struct security_operations smack_ops = {
> */
> static __init int smack_init(void)
> {
> + if (!security_module_enable(&smack_ops))
> + return 0;
> +
> printk(KERN_INFO "Smack: Initializing.\n");
>
> /*
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 358c92c..769da9a 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -986,12 +986,19 @@ static struct vfsmount *smackfs_mount;
> *
> * register the smackfs
> *
> - * Returns 0 unless the registration fails.
> + * Do not register smackfs if Smack wasn't enabled
> + * on boot.
> + *
> + * Returns true if we were not chosen on boot or if
> + * we were chosen and filesystem registration succeeded.
> */
> static int __init init_smk_fs(void)
> {
> int err;
>
> + if (!security_module_enable(&smack_ops))
> + return 0;
> +
> err = register_filesystem(&smk_fs_type);
> if (!err) {
> smackfs_mount = kern_mount(&smk_fs_type);
>
> --
>
> "Better to light a candle, than curse the darkness"
>
> Ahmed S. Darwish
> Homepage: http://darwish.07.googlepages.com
> Blog: http://darwish-07.blogspot.com
>
>
>
Casey Schaufler
[email protected]
Hi Casey,
On Wed, Mar 05, 2008 at 08:33:34AM -0800, Casey Schaufler wrote:
>
> --- "Ahmed S. Darwish" <[email protected]> wrote:
>
...
> >
> > Do not let SMACK register smackfs if it was not chosen on
> > boot. Smackfs assumes that smack hooks are registered and
> > the initial task security setup (swapper->security) is done.
>
> If the problem with initializing smackfs is because the
> locks aren't initialized why not leave the lock initializations
> in smack_init, and have them done before the check to see if the
> smack LSM is going to get used? Really, we're only talking
> about the case where a kernel is configured for testing or
> development purposes, and the lock initialization can't
> be considered a major impact in any case.
>
Beside the locking initialization issue, there's the current->security
issue. smackfs init code code access current->security in
smk_unlbl_ambient().
As you know current->security may equal Null (Oops), or point to
another LSM structure that preceeded us in registration.
The locking argument can't be applied here since we may override
the other LSM tsk->security pointer this time.
Ofcourse all of the above points can be handleded by various
if(current->security) checks + rechecking the read/write methods
of each smackfs inode, but below only two lines will fix the
problem from its roots ;):
+ if (!security_module_enable(&smack_ops))
+ return 0;
Is there a problem in the current approach that I'm not aware of ?
You have your veto in this issue at the end ;)
Thank you,
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
--- "Ahmed S. Darwish" <[email protected]> wrote:
> Hi Casey,
>
> On Wed, Mar 05, 2008 at 08:33:34AM -0800, Casey Schaufler wrote:
> >
> > --- "Ahmed S. Darwish" <[email protected]> wrote:
> >
> ...
> > >
> > > Do not let SMACK register smackfs if it was not chosen on
> > > boot. Smackfs assumes that smack hooks are registered and
> > > the initial task security setup (swapper->security) is done.
> >
> > If the problem with initializing smackfs is because the
> > locks aren't initialized why not leave the lock initializations
> > in smack_init, and have them done before the check to see if the
> > smack LSM is going to get used? Really, we're only talking
> > about the case where a kernel is configured for testing or
> > development purposes, and the lock initialization can't
> > be considered a major impact in any case.
> >
>
> Beside the locking initialization issue, there's the current->security
> issue. smackfs init code code access current->security in
> smk_unlbl_ambient().
>
> As you know current->security may equal Null (Oops), or point to
> another LSM structure that preceeded us in registration.
>
> The locking argument can't be applied here since we may override
> the other LSM tsk->security pointer this time.
>
> Ofcourse all of the above points can be handleded by various
> if(current->security) checks + rechecking the read/write methods
> of each smackfs inode, but below only two lines will fix the
> problem from its roots ;):
>
> + if (!security_module_enable(&smack_ops))
> + return 0;
>
> Is there a problem in the current approach that I'm not aware of ?
No, I made the common mistake of replying before I'd read all the
day's threads and didn't have all the information that you did.
Current LSM usage is really unfriendly to multiple "modules". I
don't see any problems with your current approach now that I've
dug into the issues a little further.
> You have your veto in this issue at the end ;)
I suppose, although that would be really stoopid when you're
doing all the hard work. Thank you.
Casey Schaufler
[email protected]
Hi!,
[
v7 log:
- Fold the SMACK bugfix here cause now it depends on the
new introduced security_ symbol.
- Do not register smackfs if Smack was not loaded
- Do not use a global to check if Smack was enabled, use
security_module_enable(ops) instead.
- Export smack_ops security operations to the rest of
SMACK code to satisfy above point.
- Remove James ACK cause patch semantics changed (could you
please reACK ?).
++:
- Add in security_module_enable() comments that it may also
be used to check if our LSM is enabled on boot.
- Remove the message `Security: %s is loaded, &lsm' from
security_module_enable cause of its new usage in above
point.
- Smack: Add a comment above init_smk_fs explaining why we
can't initialize smackfs under the security_init context.
- mmm, Will we reach v10 ?
]
-->
Add the security= boot parameter. This is done to avoid LSM
registration clashes in case of more than one bult-in module.
User can choose a security module to enable at boot. If no
security= boot parameter is specified, only the first LSM
asking for registration will be loaded. An invalid security
module name will be treated as if no module has been chosen.
LSM modules must check now if they are allowed to register
by calling security_module_enable(ops) first. Modify SELinux
and SMACK to do so.
Do not let SMACK register smackfs if it was not chosen on
boot. Smackfs assumes that smack hooks are registered and
the initial task security setup (swapper->security) is done.
Signed-off-by: Ahmed S. Darwish <[email protected]>
Reported-by: Alexey Dobriyan <[email protected]>
---
Documentation/kernel-parameters.txt | 6 ++++
include/linux/security.h | 12 +++++++++
security/dummy.c | 2 -
security/security.c | 46 +++++++++++++++++++++++++++++++++++-
security/selinux/hooks.c | 7 +++++
security/smack/smack.h | 2 +
security/smack/smack_lsm.c | 7 ++++-
security/smack/smackfs.c | 11 +++++++-
8 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9a5b665..64efbdc 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -374,6 +374,12 @@ and is between 256 and 4096 characters. It is defined in the file
possible to determine what the correct size should be.
This option provides an override for these situations.
+ security= [SECURITY] Choose a security module to enable at boot.
+ If this boot parameter is not specified, only the first
+ security module asking for security registration will be
+ loaded. An invalid security module name will be treated
+ as if no module has been chosen.
+
capability.disable=
[SECURITY] Disable capabilities. This would normally
be used only if an alternative security model is to be
diff --git a/include/linux/security.h b/include/linux/security.h
index fe52cde..801c9ad 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -42,6 +42,9 @@
extern unsigned securebits;
+/* Maximum number of letters for an LSM name string */
+#define SECURITY_NAME_MAX 10
+
struct ctl_table;
/*
@@ -117,6 +120,12 @@ struct request_sock;
/**
* struct security_operations - main security structure
*
+ * Security module identifier.
+ *
+ * @name:
+ * A string that acts as a unique identifeir for the LSM with max number
+ * of characters = SECURITY_NAME_MAX.
+ *
* Security hooks for program execution operations.
*
* @bprm_alloc_security:
@@ -1207,6 +1216,8 @@ struct request_sock;
* This is the main security structure.
*/
struct security_operations {
+ char name[SECURITY_NAME_MAX + 1];
+
int (*ptrace) (struct task_struct * parent, struct task_struct * child);
int (*capget) (struct task_struct * target,
kernel_cap_t * effective,
@@ -1466,6 +1477,7 @@ struct security_operations {
/* prototypes */
extern int security_init (void);
+extern int security_module_enable(struct security_operations *ops);
extern int register_security (struct security_operations *ops);
extern int mod_reg_security (const char *name, struct security_operations *ops);
extern struct dentry *securityfs_create_file(const char *name, mode_t mode,
diff --git a/security/dummy.c b/security/dummy.c
index 649326b..96d196f 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -979,7 +979,7 @@ static inline int dummy_key_permission(key_ref_t key_ref,
}
#endif /* CONFIG_KEYS */
-struct security_operations dummy_security_ops;
+struct security_operations dummy_security_ops = { "dummy" };
#define set_to_dummy_if_null(ops, function) \
do { \
diff --git a/security/security.c b/security/security.c
index d15e56c..deb7e00 100644
--- a/security/security.c
+++ b/security/security.c
@@ -17,6 +17,9 @@
#include <linux/kernel.h>
#include <linux/security.h>
+/* Boot-time LSM user choice */
+static spinlock_t chosen_lsm_lock;
+static char chosen_lsm[SECURITY_NAME_MAX + 1];
/* things that live in dummy.c */
extern struct security_operations dummy_security_ops;
@@ -62,18 +65,59 @@ int __init security_init(void)
}
security_ops = &dummy_security_ops;
+ spin_lock_init(&chosen_lsm_lock);
do_security_initcalls();
return 0;
}
+/* Save user chosen LSM */
+static int __init choose_lsm(char *str)
+{
+ strncpy(chosen_lsm, str, SECURITY_NAME_MAX);
+ return 1;
+}
+__setup("security=", choose_lsm);
+
+/**
+ * security_module_enable - Load given security module on boot ?
+ * @ops: a pointer to the struct security_operations that is to be checked.
+ *
+ * Each LSM must pass this method before registering its own operations
+ * to avoid security registration races. This method may also be used
+ * to check if your LSM is currently loaded.
+ *
+ * Return true if:
+ * -The passed LSM is the one chosen by user at boot time,
+ * -or user didsn't specify a specific LSM and we're the first to ask
+ * for registeration permissoin,
+ * -or the passed LSM is currently loaded.
+ * Otherwise, return false.
+ */
+int security_module_enable(struct security_operations *ops)
+{
+ int rc = 1;
+
+ spin_lock(&chosen_lsm_lock);
+
+ if (!*chosen_lsm)
+ strncpy(chosen_lsm, ops->name, SECURITY_NAME_MAX);
+ else if (strncmp(ops->name, chosen_lsm, SECURITY_NAME_MAX))
+ rc = 0;
+
+ spin_unlock(&chosen_lsm_lock);
+
+ return rc;
+}
+
/**
* register_security - registers a security framework with the kernel
* @ops: a pointer to the struct security_options that is to be registered
*
* This function is to allow a security module to register itself with the
* kernel security subsystem. Some rudimentary checking is done on the @ops
- * value passed to this function.
+ * value passed to this function. You'll need to check first if your LSM
+ * is allowed to register its @ops by calling security_module_enable(@ops).
*
* If there is already a security module registered with the kernel,
* an error will be returned. Otherwise 0 is returned on success.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 75c2e99..49709a4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5219,6 +5219,8 @@ static int selinux_key_permission(key_ref_t key_ref,
#endif
static struct security_operations selinux_ops = {
+ .name = "selinux",
+
.ptrace = selinux_ptrace,
.capget = selinux_capget,
.capset_check = selinux_capset_check,
@@ -5405,6 +5407,11 @@ static __init int selinux_init(void)
{
struct task_security_struct *tsec;
+ if (!security_module_enable(&selinux_ops)) {
+ selinux_enabled = 0;
+ return 0;
+ }
+
if (!selinux_enabled) {
printk(KERN_INFO "SELinux: Disabled at boot.\n");
return 0;
diff --git a/security/smack/smack.h b/security/smack/smack.h
index a21a0e9..c444f48 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -15,6 +15,7 @@
#include <linux/capability.h>
#include <linux/spinlock.h>
+#include <linux/security.h>
#include <net/netlabel.h>
/*
@@ -195,6 +196,7 @@ extern struct smack_known smack_known_star;
extern struct smack_known smack_known_unset;
extern struct smk_list_entry *smack_list;
+extern struct security_operations smack_ops;
/*
* Stricly for CIPSO level manipulation.
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 770eb06..afa7967 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2433,7 +2433,9 @@ static void smack_release_secctx(char *secdata, u32 seclen)
{
}
-static struct security_operations smack_ops = {
+struct security_operations smack_ops = {
+ .name = "smack",
+
.ptrace = smack_ptrace,
.capget = cap_capget,
.capset_check = cap_capset_check,
@@ -2566,6 +2568,9 @@ static struct security_operations smack_ops = {
*/
static __init int smack_init(void)
{
+ if (!security_module_enable(&smack_ops))
+ return 0;
+
printk(KERN_INFO "Smack: Initializing.\n");
/*
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 358c92c..effd3de 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -986,12 +986,21 @@ static struct vfsmount *smackfs_mount;
*
* register the smackfs
*
- * Returns 0 unless the registration fails.
+ * Do not register smackfs if Smack wasn't enabled
+ * on boot. We can not put this method normally under the
+ * smack_init() code path since the security subsystem get
+ * initialized before the vfs caches.
+ *
+ * Returns true if we were not chosen on boot or if
+ * we were chosen and filesystem registration succeeded.
*/
static int __init init_smk_fs(void)
{
int err;
+ if (!security_module_enable(&smack_ops))
+ return 0;
+
err = register_filesystem(&smk_fs_type);
if (!err) {
smackfs_mount = kern_mount(&smk_fs_type);
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com