lsm: add the inode_free_security_rcu() LSM implementation hook

The LSM framework has an existing inode_free_security() hook which
is used by LSMs that manage state associated with an inode, but
due to the use of RCU to protect the inode, special care must be
taken to ensure that the LSMs do not fully release the inode state
until it is safe from a RCU perspective.

This patch implements a new inode_free_security_rcu() implementation
hook which is called when it is safe to free the LSM's internal inode
state.  Unfortunately, this new hook does not have access to the inode
itself as it may already be released, so the existing
inode_free_security() hook is retained for those LSMs which require
access to the inode.

Cc: stable@vger.kernel.org
Reported-by: syzbot+5446fbf332b0602ede0b@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/00000000000076ba3b0617f65cc8@google.com
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 63e2656d1d5679e4ce28a3408777d4a059cff382..520730fe2d949b1b412c8335be1be4dfa03a9d9b 100644 (file)
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -114,6 +114,7 @@ LSM_HOOK(int, 0, path_notify, const struct path *path, u64 mask,
         unsigned int obj_type)
 LSM_HOOK(int, 0, inode_alloc_security, struct inode *inode)
 LSM_HOOK(void, LSM_RET_VOID, inode_free_security, struct inode *inode)
+LSM_HOOK(void, LSM_RET_VOID, inode_free_security_rcu, void *inode_security)
 LSM_HOOK(int, -EOPNOTSUPP, inode_init_security, struct inode *inode,
         struct inode *dir, const struct qstr *qstr, struct xattr *xattrs,
         int *xattr_count)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c51e24d24d1e9ec64d0713016213a8ba02c8da45..3c323ca213d42cfd782a0c350590e4a2d5dc23d5 100644 (file)
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -223,7 +223,7 @@ static inline void ima_inode_set_iint(const struct inode *inode,
 
 struct ima_iint_cache *ima_iint_find(struct inode *inode);
 struct ima_iint_cache *ima_inode_get(struct inode *inode);
-void ima_inode_free(struct inode *inode);
+void ima_inode_free_rcu(void *inode_security);
 void __init ima_iintcache_init(void);
 
 extern const int read_idmap[];

diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index e23412a2c56b092173d7f0db111d32c6fce37c67..00b249101f98d39a07e0d5a3c9357496bbf438ae 100644 (file)
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -109,22 +109,18 @@ struct ima_iint_cache *ima_inode_get(struct inode *inode)
 }
 
 /**
- * ima_inode_free - Called on inode free
- * @inode: Pointer to the inode
+ * ima_inode_free_rcu - Called to free an inode via a RCU callback
+ * @inode_security: The inode->i_security pointer
  *
- * Free the iint associated with an inode.
+ * Free the IMA data associated with an inode.
  */
-void ima_inode_free(struct inode *inode)
+void ima_inode_free_rcu(void *inode_security)
 {
-       struct ima_iint_cache *iint;
-
-       if (!IS_IMA(inode))
-               return;
-
-       iint = ima_iint_find(inode);
-       ima_inode_set_iint(inode, NULL);
+       struct ima_iint_cache **iint_p = inode_security + ima_blob_sizes.lbs_inode;
 
-       ima_iint_free(iint);
+       /* *iint_p should be NULL if !IS_IMA(inode) */
+       if (*iint_p)
+               ima_iint_free(*iint_p);
 }
 
 static void ima_iint_init_once(void *foo)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index f04f43af651c8e56754ab3e32cde9adc6d18915b..5b3394864b218e0388fd24dd7131bbe5292784f6 100644 (file)
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1193,7 +1193,7 @@ static struct security_hook_list ima_hooks[] __ro_after_init = {
 #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
        LSM_HOOK_INIT(kernel_module_request, ima_kernel_module_request),
 #endif
-       LSM_HOOK_INIT(inode_free_security, ima_inode_free),
+       LSM_HOOK_INIT(inode_free_security_rcu, ima_inode_free_rcu),
 };
 
 static const struct lsm_id ima_lsmid = {

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 7877a64cc6b87c0930e679ab07ee469c5bec22e5..0804f76a67be2e65d27d23ae835c18f967e371ea 100644 (file)
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1207,13 +1207,16 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 
 /* Inode hooks */
 
-static void hook_inode_free_security(struct inode *const inode)
+static void hook_inode_free_security_rcu(void *inode_security)
 {
+       struct landlock_inode_security *inode_sec;
+
        /*
         * All inodes must already have been untied from their object by
         * release_inode() or hook_sb_delete().
         */
-       WARN_ON_ONCE(landlock_inode(inode)->object);
+       inode_sec = inode_security + landlock_blob_sizes.lbs_inode;
+       WARN_ON_ONCE(inode_sec->object);
 }
 
 /* Super-block hooks */
@@ -1637,7 +1640,7 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
 }
 
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
-       LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
+       LSM_HOOK_INIT(inode_free_security_rcu, hook_inode_free_security_rcu),
 
        LSM_HOOK_INIT(sb_delete, hook_sb_delete),
        LSM_HOOK_INIT(sb_mount, hook_sb_mount),

diff --git a/security/security.c b/security/security.c
index b316e6586be200d35302890f8c45a16afdd1f482..611d3c124ba6195c9d8740d1d8c34bcb9958c1ad 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -1609,9 +1609,8 @@ int security_inode_alloc(struct inode *inode)
 
 static void inode_free_by_rcu(struct rcu_head *head)
 {
-       /*
-        * The rcu head is at the start of the inode blob
-        */
+       /* The rcu head is at the start of the inode blob */
+       call_void_hook(inode_free_security_rcu, head);
        kmem_cache_free(lsm_inode_cache, head);
 }
 
@@ -1619,23 +1618,24 @@ static void inode_free_by_rcu(struct rcu_head *head)
  * security_inode_free() - Free an inode's LSM blob
  * @inode: the inode
  *
- * Deallocate the inode security structure and set @inode->i_security to NULL.
+ * Release any LSM resources associated with @inode, although due to the
+ * inode's RCU protections it is possible that the resources will not be
+ * fully released until after the current RCU grace period has elapsed.
+ *
+ * It is important for LSMs to note that despite being present in a call to
+ * security_inode_free(), @inode may still be referenced in a VFS path walk
+ * and calls to security_inode_permission() may be made during, or after,
+ * a call to security_inode_free().  For this reason the inode->i_security
+ * field is released via a call_rcu() callback and any LSMs which need to
+ * retain inode state for use in security_inode_permission() should only
+ * release that state in the inode_free_security_rcu() LSM hook callback.
  */
 void security_inode_free(struct inode *inode)
 {
        call_void_hook(inode_free_security, inode);
-       /*
-        * The inode may still be referenced in a path walk and
-        * a call to security_inode_permission() can be made
-        * after inode_free_security() is called. Ideally, the VFS
-        * wouldn't do this, but fixing that is a much harder
-        * job. For now, simply free the i_security via RCU, and
-        * leave the current inode->i_security pointer intact.
-        * The inode will be freed after the RCU grace period too.
-        */
-       if (inode->i_security)
-               call_rcu((struct rcu_head *)inode->i_security,
-                        inode_free_by_rcu);
+       if (!inode->i_security)
+               return;
+       call_rcu((struct rcu_head *)inode->i_security, inode_free_by_rcu);
 }
 
 /**