LSM: Infrastructure management of the file security

Move management of the file->f_security blob out of the
individual security modules and into the infrastructure.
The modules no longer allocate or free the data, instead
they tell the infrastructure how much space they require.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[kees: adjusted for ordered init series]
Signed-off-by: Kees Cook <keescook@chromium.org>

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index dd33666567bc93ef3f9473e84e19baae699fbee3..e8cef019b64586fe8c10b58c35e13018181fe7ea 100644 (file)
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2032,6 +2032,7 @@ struct security_hook_list {
  */
 struct lsm_blob_sizes {
        int     lbs_cred;
+       int     lbs_file;
 };
 
 /*

diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h
index 4c2c8ac8842f6d8c05c4480fa2680b4ba9ab75df..8be09208cf7c119ebd0f20e494c9f466fc74c15a 100644 (file)
--- a/security/apparmor/include/file.h
+++ b/security/apparmor/include/file.h
@@ -32,7 +32,10 @@ struct path;
                                 AA_MAY_CHMOD | AA_MAY_CHOWN | AA_MAY_LOCK | \
                                 AA_EXEC_MMAP | AA_MAY_LINK)
 
-#define file_ctx(X) ((struct aa_file_ctx *)(X)->f_security)
+static inline struct aa_file_ctx *file_ctx(struct file *file)
+{
+       return file->f_security + apparmor_blob_sizes.lbs_file;
+}
 
 /* struct aa_file_ctx - the AppArmor context the file was opened in
  * @lock: lock to update the ctx

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index d5e4a384f2050ff9c9487996e54d24bbf51ac266..6821187b06adfb02e0462dd7a9020c3c3cf46bf4 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -434,21 +434,21 @@ static int apparmor_file_open(struct file *file)
 
 static int apparmor_file_alloc_security(struct file *file)
 {
-       int error = 0;
-
-       /* freed by apparmor_file_free_security */
+       struct aa_file_ctx *ctx = file_ctx(file);
        struct aa_label *label = begin_current_label_crit_section();
-       file->f_security = aa_alloc_file_ctx(label, GFP_KERNEL);
-       if (!file_ctx(file))
-               error = -ENOMEM;
-       end_current_label_crit_section(label);
 
-       return error;
+       spin_lock_init(&ctx->lock);
+       rcu_assign_pointer(ctx->label, aa_get_label(label));
+       end_current_label_crit_section(label);
+       return 0;
 }
 
 static void apparmor_file_free_security(struct file *file)
 {
-       aa_free_file_ctx(file_ctx(file));
+       struct aa_file_ctx *ctx = file_ctx(file);
+
+       if (ctx)
+               aa_put_label(rcu_access_pointer(ctx->label));
 }
 
 static int common_file_perm(const char *op, struct file *file, u32 mask)
@@ -1156,6 +1156,7 @@ static int apparmor_inet_conn_request(struct sock *sk, struct sk_buff *skb,
  */
 struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
        .lbs_cred = sizeof(struct aa_task_ctx *),
+       .lbs_file = sizeof(struct aa_file_ctx),
 };
 
 static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {

diff --git a/security/security.c b/security/security.c
index 09be8ce007a25438bca3616eccce4f388a3318bd..f32d7d2075c698be40bff3ef92076a7064ddef4a 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -40,6 +40,8 @@
 struct security_hook_heads security_hook_heads __lsm_ro_after_init;
 static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
 
+static struct kmem_cache *lsm_file_cache;
+
 char *lsm_names;
 static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
 
@@ -158,6 +160,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
                return;
 
        lsm_set_blob_size(&needed->lbs_cred, &blob_sizes.lbs_cred);
+       lsm_set_blob_size(&needed->lbs_file, &blob_sizes.lbs_file);
 }
 
 /* Prepare LSM for initialization. */
@@ -279,6 +282,15 @@ static void __init ordered_lsm_init(void)
                prepare_lsm(*lsm);
 
        init_debug("cred blob size     = %d\n", blob_sizes.lbs_cred);
+       init_debug("file blob size     = %d\n", blob_sizes.lbs_file);
+
+       /*
+        * Create any kmem_caches needed for blobs
+        */
+       if (blob_sizes.lbs_file)
+               lsm_file_cache = kmem_cache_create("lsm_file_cache",
+                                                  blob_sizes.lbs_file, 0,
+                                                  SLAB_PANIC, NULL);
 
        for (lsm = ordered_lsms; *lsm; lsm++)
                initialize_lsm(*lsm);
@@ -448,6 +460,27 @@ void __init lsm_early_cred(struct cred *cred)
                panic("%s: Early cred alloc failed.\n", __func__);
 }
 
+/**
+ * lsm_file_alloc - allocate a composite file blob
+ * @file: the file that needs a blob
+ *
+ * Allocate the file blob for all the modules
+ *
+ * Returns 0, or -ENOMEM if memory can't be allocated.
+ */
+static int lsm_file_alloc(struct file *file)
+{
+       if (!lsm_file_cache) {
+               file->f_security = NULL;
+               return 0;
+       }
+
+       file->f_security = kmem_cache_zalloc(lsm_file_cache, GFP_KERNEL);
+       if (file->f_security == NULL)
+               return -ENOMEM;
+       return 0;
+}
+
 /*
  * Hook list operation macros.
  *
@@ -1144,12 +1177,27 @@ int security_file_permission(struct file *file, int mask)
 
 int security_file_alloc(struct file *file)
 {
-       return call_int_hook(file_alloc_security, 0, file);
+       int rc = lsm_file_alloc(file);
+
+       if (rc)
+               return rc;
+       rc = call_int_hook(file_alloc_security, 0, file);
+       if (unlikely(rc))
+               security_file_free(file);
+       return rc;
 }
 
 void security_file_free(struct file *file)
 {
+       void *blob;
+
        call_void_hook(file_free_security, file);
+
+       blob = file->f_security;
+       if (blob) {
+               file->f_security = NULL;
+               kmem_cache_free(lsm_file_cache, blob);
+       }
 }
 
 int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
@@ -1267,7 +1315,7 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
                return rc;
 
        rc = call_int_hook(cred_alloc_blank, 0, cred, gfp);
-       if (rc)
+       if (unlikely(rc))
                security_cred_free(cred);
        return rc;
 }
@@ -1288,7 +1336,7 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp)
                return rc;
 
        rc = call_int_hook(cred_prepare, 0, new, old, gfp);
-       if (rc)
+       if (unlikely(rc))
                security_cred_free(new);
        return rc;
 }

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 620be0367c0b6ba9e68f2d242b3ab1f3dcc107c4..632813821da6e40dd00386489d1e12081985840d 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -146,7 +146,6 @@ static int __init checkreqprot_setup(char *str)
 __setup("checkreqprot=", checkreqprot_setup);
 
 static struct kmem_cache *sel_inode_cache;
-static struct kmem_cache *file_security_cache;
 
 /**
  * selinux_secmark_enabled - Check to see if SECMARK is currently enabled
@@ -378,27 +377,15 @@ static void inode_free_security(struct inode *inode)
 
 static int file_alloc_security(struct file *file)
 {
-       struct file_security_struct *fsec;
+       struct file_security_struct *fsec = selinux_file(file);
        u32 sid = current_sid();
 
-       fsec = kmem_cache_zalloc(file_security_cache, GFP_KERNEL);
-       if (!fsec)
-               return -ENOMEM;
-
        fsec->sid = sid;
        fsec->fown_sid = sid;
-       file->f_security = fsec;
 
        return 0;
 }
 
-static void file_free_security(struct file *file)
-{
-       struct file_security_struct *fsec = selinux_file(file);
-       file->f_security = NULL;
-       kmem_cache_free(file_security_cache, fsec);
-}
-
 static int superblock_alloc_security(struct super_block *sb)
 {
        struct superblock_security_struct *sbsec;
@@ -3345,11 +3332,6 @@ static int selinux_file_alloc_security(struct file *file)
        return file_alloc_security(file);
 }
 
-static void selinux_file_free_security(struct file *file)
-{
-       file_free_security(file);
-}
-
 /*
  * Check whether a task has the ioctl permission and cmd
  * operation to an inode.
@@ -6646,6 +6628,7 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
 
 struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
        .lbs_cred = sizeof(struct task_security_struct),
+       .lbs_file = sizeof(struct file_security_struct),
 };
 
 static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
@@ -6717,7 +6700,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 
        LSM_HOOK_INIT(file_permission, selinux_file_permission),
        LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
-       LSM_HOOK_INIT(file_free_security, selinux_file_free_security),
        LSM_HOOK_INIT(file_ioctl, selinux_file_ioctl),
        LSM_HOOK_INIT(mmap_file, selinux_mmap_file),
        LSM_HOOK_INIT(mmap_addr, selinux_mmap_addr),
@@ -6902,9 +6884,6 @@ static __init int selinux_init(void)
        sel_inode_cache = kmem_cache_create("selinux_inode_security",
                                            sizeof(struct inode_security_struct),
                                            0, SLAB_PANIC, NULL);
-       file_security_cache = kmem_cache_create("selinux_file_security",
-                                           sizeof(struct file_security_struct),
-                                           0, SLAB_PANIC, NULL);
        avc_init();
 
        avtab_cache_init();

diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index e0ac2992e059e5d688dbccda5e54de91207fcfe0..96374dbf4acec4faac578409e686e041515340de 100644 (file)
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -167,7 +167,7 @@ static inline struct task_security_struct *selinux_cred(const struct cred *cred)
 
 static inline struct file_security_struct *selinux_file(const struct file *file)
 {
-       return file->f_security;
+       return file->f_security + selinux_blob_sizes.lbs_file;
 }
 
 #endif /* _SELINUX_OBJSEC_H_ */

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 50854969a391986d9d36deb4257c49418c1090c2..2007d38d0e46d0c385773197fe841d36cf072481 100644 (file)
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -364,7 +364,8 @@ static inline struct task_smack *smack_cred(const struct cred *cred)
 
 static inline struct smack_known **smack_file(const struct file *file)
 {
-       return (struct smack_known **)&file->f_security;
+       return (struct smack_known **)(file->f_security +
+                                      smack_blob_sizes.lbs_file);
 }
 
 /*

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 8f72641f94aba1dda50ac81352a5a98a6f8c51a5..7c76668ea3a6782bc9b7ff4c7418c14e95806e26 100644 (file)
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1495,18 +1495,6 @@ static int smack_file_alloc_security(struct file *file)
        return 0;
 }
 
-/**
- * smack_file_free_security - clear a file security blob
- * @file: the object
- *
- * The security blob for a file is a pointer to the master
- * label list, so no memory is freed.
- */
-static void smack_file_free_security(struct file *file)
-{
-       file->f_security = NULL;
-}
-
 /**
  * smack_file_ioctl - Smack check on ioctls
  * @file: the object
@@ -4559,6 +4547,7 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode,
 
 struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
        .lbs_cred = sizeof(struct task_smack),
+       .lbs_file = sizeof(struct smack_known *),
 };
 
 static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
@@ -4595,7 +4584,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
        LSM_HOOK_INIT(inode_getsecid, smack_inode_getsecid),
 
        LSM_HOOK_INIT(file_alloc_security, smack_file_alloc_security),
-       LSM_HOOK_INIT(file_free_security, smack_file_free_security),
        LSM_HOOK_INIT(file_ioctl, smack_file_ioctl),
        LSM_HOOK_INIT(file_lock, smack_file_lock),
        LSM_HOOK_INIT(file_fcntl, smack_file_fcntl),