LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: "Ahmed S. Darwish" <darwish.07@gmail.com>
Cc: jmorris@namei.org, sds@tycho.nsa.gov, casey@schaufler-ca.com,
	bunk@kernel.org, chrisw@sous-sol.org, eparis@parisplace.org,
	adobriyan@sw.ru, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH -v5 -mm] LSM: Add security= boot parameter
Date: Wed, 5 Mar 2008 14:29:48 -0800
Message-ID: <20080305142948.3d391d84.akpm@linux-foundation.org> (raw)
In-Reply-To: <20080304030407.GA25686@ubuntu>

On Tue, 4 Mar 2008 05:04:07 +0200
"Ahmed S. Darwish" <darwish.07@gmail.com> wrote:

> Hi!,
> 
> [ 
> Fix stuff mentioned by James in parent mail:
> - use spinlocks instead of atomic counter (yes, this is clearer).
> - remove redundant BUG_ON
> - don't let LSMs loudly complain when they aren't chosen.
> ]
> 
> -->
> 
> 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.
> 
> ...
>  
> +/* Maximum number of letters for an LSM name string */
> +#define SECURITY_NAME_MAX	10

Is this long enough?

>  struct ctl_table;
>  struct audit_krule;
>  
>  ...
>
> -struct security_operations dummy_security_ops;
> +struct security_operations dummy_security_ops = { "dummy" };

Please don't rely upon the layout of data structures in this manner.  Use
".name = ".

>  
>  #define set_to_dummy_if_null(ops, function)				\
>  	do {								\
> diff --git a/security/security.c b/security/security.c
> index 1bf2ee4..def9fc0 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);

Please remove this and use compile-time initialisation with DEFINE_SPINLOCK.

Do we actually need the lock?  This code is only called at boot-time if I
understand it correctly?

Can chosen_lsm[] be __initdata?

>  	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;
> +}

I believe this can be __init.

> +	if (!security_module_enable(&selinux_ops)) {
> +		selinux_enabled = 0;
> +		return 0;
> +	}
> +
>
> ...
>
>  static __init int smack_init(void)
>  {
> +	if (!security_module_enable(&smack_ops))
> +		return 0;
> +
>  	printk(KERN_INFO "Smack:  Initializing.\n");
>  
>  	/*

hm.  selinux has a global selinux_enabled knob, but smack seems to be able
to get by without one.  +1 for smack ;)




  parent reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-01 19:07 [RFC PATCH -mm] LSM: Add lsm= " Ahmed S. Darwish
2008-03-01 20:28 ` Casey Schaufler
2008-03-01 21:11   ` Adrian Bunk
2008-03-01 21:29     ` Casey Schaufler
2008-03-01 23:27       ` [PATCH -v2 -mm] LSM: Add security= " Ahmed S. Darwish
2008-03-02  3:41         ` Casey Schaufler
2008-03-02  7:55           ` Ahmed S. Darwish
2008-03-02  7:49         ` Ahmed S. Darwish
2008-03-02 10:59           ` [PATCH -v3 " Ahmed S. Darwish
2008-03-02 18:37             ` Casey Schaufler
2008-03-03  8:29             ` James Morris
2008-03-03 15:35               ` Ahmed S. Darwish
2008-03-03 15:54                 ` Stephen Smalley
2008-03-03 21:24                   ` [PATCH -v4 " Ahmed S. Darwish
2008-03-03 22:16                     ` James Morris
2008-03-04  3:04                       ` [PATCH -v5 " Ahmed S. Darwish
2008-03-04  4:07                         ` James Morris
2008-03-05 22:29                         ` Andrew Morton [this message]
2008-03-05 22:56                           ` Ahmed S. Darwish
2008-03-05 23:06                             ` Ahmed S. Darwish
2008-03-05 22:56                           ` James Morris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080305142948.3d391d84.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=adobriyan@sw.ru \
    --cc=bunk@kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=chrisw@sous-sol.org \
    --cc=darwish.07@gmail.com \
    --cc=eparis@parisplace.org \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=sds@tycho.nsa.gov \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lkml.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lkml.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lkml.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lkml.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lkml.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lkml.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lkml.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lkml.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lkml.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lkml.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lkml.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lkml.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git