[PATCH v2] LSM: Make security_hook_heads a local variable.

[Casey Schaufler]

On 3/24/2017 4:16 AM, Tetsuo Handa wrote:
> Since commit 3dfc9b02864b19f4 ("LSM: Initialize security_hook_heads upon
> registration.") changed to access security_hook_heads as an array of
> "struct list_head", we no longer need to pass address of each member inside
> "struct security_hook_heads". Therefore, we can make security_hook_heads
> a local variable by passing index number inside "struct list_head" array.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel at I-love.SAKURA.ne.jp>
> Cc: Kees Cook <keescook at chromium.org>
> Cc: Paul Moore <paul at paul-moore.com>
> Cc: Stephen Smalley <sds at tycho.nsa.gov>
> Cc: Casey Schaufler <casey at schaufler-ca.com>
> Cc: James Morris <james.l.morris at oracle.com>
> ---
>  include/linux/lsm_hooks.h |  6 +++---
>  security/security.c       | 13 +++++++++++--
>  2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 1aa6333..54191cf 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1877,8 +1877,8 @@ struct security_hook_heads {
>   */
>  struct security_hook_list {
>  	struct list_head		list;
> -	struct list_head		*head;
>  	union security_list_options	hook;
> +	const unsigned int		idx;
>  	char				*lsm;
>  };
>  
> @@ -1889,9 +1889,9 @@ struct security_hook_list {
>   * text involved.
>   */
>  #define LSM_HOOK_INIT(HEAD, HOOK) \
> -	{ .head = &security_hook_heads.HEAD, .hook = { .HEAD = HOOK } }
> +	{ .idx = offsetof(struct security_hook_heads, HEAD) / \
> +		sizeof(struct list_head), .hook = { .HEAD = HOOK } }

You're gaining making security_hook_heads static,
but you're loosing the type checking. I found (much
to my surprise at times) that the type checking is
very helpful even with existing modules. I expect
some module developers to get bitten by this.


>  
> -extern struct security_hook_heads security_hook_heads;
>  extern char *lsm_names;
>  
>  extern void security_add_hooks(struct security_hook_list *hooks, int count,
> diff --git a/security/security.c b/security/security.c
> index 2f15488..45af8fb 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -32,7 +32,7 @@
>  /* Maximum number of letters for an LSM name string */
>  #define SECURITY_NAME_MAX	10
>  
> -struct security_hook_heads security_hook_heads __lsm_ro_after_init;
> +static struct security_hook_heads security_hook_heads __lsm_ro_after_init;
>  char *lsm_names;
>  /* Boot-time LSM user choice */
>  static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
> @@ -133,10 +133,19 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  				char *lsm)
>  {
>  	int i;
> +	unsigned int idx;
> +	struct list_head *list = (struct list_head *) &security_hook_heads;
>  
>  	for (i = 0; i < count; i++) {
>  		hooks[i].lsm = lsm;
> -		list_add_tail_rcu(&hooks[i].list, hooks[i].head);
> +		/*
> +		 * LSM_HOOK_INIT() must have set correct idx value.
> +		 * But just in case something went wrong.
> +		 */
> +		idx = hooks[i].idx;
> +		BUG_ON(idx >= sizeof(struct security_hook_heads) /
> +		       sizeof(struct list_head));
> +		list_add_tail_rcu(&hooks[i].list, &list[idx]);
>  	}
>  	if (lsm_append(lsm, &lsm_names) < 0)
>  		panic("%s - Cannot get early memory.\n", __func__);

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html