diff mbox series

[v3,13/25] security: Introduce inode_post_removexattr hook

Message ID 20230904133415.1799503-14-roberto.sassu@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series security: Move IMA and EVM to the LSM infrastructure | expand

Commit Message

Roberto Sassu Sept. 4, 2023, 1:34 p.m. UTC 
From: Roberto Sassu <roberto.sassu@huawei.com>

In preparation for moving IMA and EVM to the LSM infrastructure, introduce
the inode_post_removexattr hook.

It is useful for EVM to recalculate the HMAC on the remaining file xattrs,
and other file metadata, after it verified the HMAC of current file
metadata with the inode_removexattr hook.

LSMs should use the new hook instead of inode_removexattr, when they need
to know that the operation was done successfully (not known in
inode_removexattr). The new hook cannot return an error and cannot cause
the operation to be reverted.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/xattr.c                    |  9 +++++----
 include/linux/lsm_hook_defs.h |  2 ++
 include/linux/security.h      |  5 +++++
 security/security.c           | 14 ++++++++++++++
 4 files changed, 26 insertions(+), 4 deletions(-)

Comments

Stefan Berger Sept. 5, 2023, 5:55 p.m. UTC | #1 
On 9/4/23 09:34, Roberto Sassu wrote:

> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> the inode_post_removexattr hook.
>
> It is useful for EVM to recalculate the HMAC on the remaining file xattrs,
> and other file metadata, after it verified the HMAC of current file
> metadata with the inode_removexattr hook.
>
> LSMs should use the new hook instead of inode_removexattr, when they need
> to know that the operation was done successfully (not known in
> inode_removexattr). The new hook cannot return an error and cannot cause
> the operation to be reverted.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>   fs/xattr.c                    |  9 +++++----
>   include/linux/lsm_hook_defs.h |  2 ++
>   include/linux/security.h      |  5 +++++
>   security/security.c           | 14 ++++++++++++++
>   4 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index e7bbb7f57557..4a0280295686 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -552,11 +552,12 @@ __vfs_removexattr_locked(struct mnt_idmap *idmap,
>   		goto out;
>   
>   	error = __vfs_removexattr(idmap, dentry, name);
> +	if (error)
> +		goto out;
>   
> -	if (!error) {
> -		fsnotify_xattr(dentry);
> -		evm_inode_post_removexattr(dentry, name);
> -	}
> +	fsnotify_xattr(dentry);
> +	security_inode_post_removexattr(dentry, name);
> +	evm_inode_post_removexattr(dentry, name);
>   
>   out:
>   	return error;
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 995d30336cfa..1153e7163b8b 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -148,6 +148,8 @@ LSM_HOOK(int, 0, inode_getxattr, struct dentry *dentry, const char *name)
>   LSM_HOOK(int, 0, inode_listxattr, struct dentry *dentry)
>   LSM_HOOK(int, 0, inode_removexattr, struct mnt_idmap *idmap,
>   	 struct dentry *dentry, const char *name)
> +LSM_HOOK(void, LSM_RET_VOID, inode_post_removexattr, struct dentry *dentry,
> +	 const char *name)
>   LSM_HOOK(int, 0, inode_set_acl, struct mnt_idmap *idmap,
>   	 struct dentry *dentry, const char *acl_name, struct posix_acl *kacl)
>   LSM_HOOK(int, 0, inode_get_acl, struct mnt_idmap *idmap,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 820899db5276..665bba3e0081 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -374,6 +374,7 @@ int security_inode_getxattr(struct dentry *dentry, const char *name);
>   int security_inode_listxattr(struct dentry *dentry);
>   int security_inode_removexattr(struct mnt_idmap *idmap,
>   			       struct dentry *dentry, const char *name);
> +void security_inode_post_removexattr(struct dentry *dentry, const char *name);
>   int security_inode_need_killpriv(struct dentry *dentry);
>   int security_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
>   int security_inode_getsecurity(struct mnt_idmap *idmap,
> @@ -919,6 +920,10 @@ static inline int security_inode_removexattr(struct mnt_idmap *idmap,
>   	return cap_inode_removexattr(idmap, dentry, name);
>   }
>   
> +static inline void security_inode_post_removexattr(struct dentry *dentry,
> +						   const char *name)
> +{ }
> +
>   static inline int security_inode_need_killpriv(struct dentry *dentry)
>   {
>   	return cap_inode_need_killpriv(dentry);
> diff --git a/security/security.c b/security/security.c
> index 764a6f28b3b9..3947159ba5e9 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2354,6 +2354,20 @@ int security_inode_removexattr(struct mnt_idmap *idmap,
>   	return evm_inode_removexattr(idmap, dentry, name);
>   }
>   
> +/**
> + * security_inode_post_removexattr() - Update the inode after a removexattr op
> + * @dentry: file
> + * @name: xattr name
> + *
> + * Update the inode after a successful removexattr operation.
> + */
> +void security_inode_post_removexattr(struct dentry *dentry, const char *name)
> +{
> +	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> +		return;
> +	call_void_hook(inode_post_removexattr, dentry, name);
> +}
> +
>   /**
>    * security_inode_need_killpriv() - Check if security_inode_killpriv() required
>    * @dentry: associated dentry


Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Mimi Zohar Oct. 12, 2023, 12:09 a.m. UTC | #2 
On Mon, 2023-09-04 at 15:34 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> the inode_post_removexattr hook.
> 
> It is useful for EVM to recalculate the HMAC on the remaining file xattrs,
> and other file metadata, after it verified the HMAC of current file
> metadata with the inode_removexattr hook.
> 
> LSMs should use the new hook instead of inode_removexattr, when they need
> to know that the operation was done successfully (not known in
> inode_removexattr). 

> The new hook cannot return an error and cannot cause
> the operation to be reverted.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Refer to the patch description comments for 12/25.

Otherwise,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
diff mbox series

Patch

diff --git a/fs/xattr.c b/fs/xattr.c
index e7bbb7f57557..4a0280295686 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -552,11 +552,12 @@  __vfs_removexattr_locked(struct mnt_idmap *idmap,
 		goto out;
 
 	error = __vfs_removexattr(idmap, dentry, name);
+	if (error)
+		goto out;
 
-	if (!error) {
-		fsnotify_xattr(dentry);
-		evm_inode_post_removexattr(dentry, name);
-	}
+	fsnotify_xattr(dentry);
+	security_inode_post_removexattr(dentry, name);
+	evm_inode_post_removexattr(dentry, name);
 
 out:
 	return error;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 995d30336cfa..1153e7163b8b 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -148,6 +148,8 @@  LSM_HOOK(int, 0, inode_getxattr, struct dentry *dentry, const char *name)
 LSM_HOOK(int, 0, inode_listxattr, struct dentry *dentry)
 LSM_HOOK(int, 0, inode_removexattr, struct mnt_idmap *idmap,
 	 struct dentry *dentry, const char *name)
+LSM_HOOK(void, LSM_RET_VOID, inode_post_removexattr, struct dentry *dentry,
+	 const char *name)
 LSM_HOOK(int, 0, inode_set_acl, struct mnt_idmap *idmap,
 	 struct dentry *dentry, const char *acl_name, struct posix_acl *kacl)
 LSM_HOOK(int, 0, inode_get_acl, struct mnt_idmap *idmap,
diff --git a/include/linux/security.h b/include/linux/security.h
index 820899db5276..665bba3e0081 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -374,6 +374,7 @@  int security_inode_getxattr(struct dentry *dentry, const char *name);
 int security_inode_listxattr(struct dentry *dentry);
 int security_inode_removexattr(struct mnt_idmap *idmap,
 			       struct dentry *dentry, const char *name);
+void security_inode_post_removexattr(struct dentry *dentry, const char *name);
 int security_inode_need_killpriv(struct dentry *dentry);
 int security_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
 int security_inode_getsecurity(struct mnt_idmap *idmap,
@@ -919,6 +920,10 @@  static inline int security_inode_removexattr(struct mnt_idmap *idmap,
 	return cap_inode_removexattr(idmap, dentry, name);
 }
 
+static inline void security_inode_post_removexattr(struct dentry *dentry,
+						   const char *name)
+{ }
+
 static inline int security_inode_need_killpriv(struct dentry *dentry)
 {
 	return cap_inode_need_killpriv(dentry);
diff --git a/security/security.c b/security/security.c
index 764a6f28b3b9..3947159ba5e9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2354,6 +2354,20 @@  int security_inode_removexattr(struct mnt_idmap *idmap,
 	return evm_inode_removexattr(idmap, dentry, name);
 }
 
+/**
+ * security_inode_post_removexattr() - Update the inode after a removexattr op
+ * @dentry: file
+ * @name: xattr name
+ *
+ * Update the inode after a successful removexattr operation.
+ */
+void security_inode_post_removexattr(struct dentry *dentry, const char *name)
+{
+	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
+		return;
+	call_void_hook(inode_post_removexattr, dentry, name);
+}
+
 /**
  * security_inode_need_killpriv() - Check if security_inode_killpriv() required
  * @dentry: associated dentry