Re: [PATCH v5 04/12] ima: Move ima_reset_appraise_flags() call to post hooks
From: Mimi Zohar <zohar@linux.ibm.com>
Date: 2021-04-26 19:49:57
Also in:
linux-fsdevel, linux-integrity, lkml
On Wed, 2021-04-07 at 09:17 -0700, Casey Schaufler wrote:
quoted
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 565e33ff19d0..1f029e4c8d7f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c@@ -577,21 +577,40 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, if (result == 1) { if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST)) return -EINVAL; - ima_reset_appraise_flags(d_backing_inode(dentry), - xvalue->type == EVM_IMA_XATTR_DIGSIG); result = 0; } return result; } +void ima_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, + const void *xattr_value, size_t xattr_value_len) +{ + const struct evm_ima_xattr_data *xvalue = xattr_value; + int result; + + result = ima_protect_xattr(dentry, xattr_name, xattr_value, + xattr_value_len); + if (result == 1) + ima_reset_appraise_flags(d_backing_inode(dentry), + xvalue->type == EVM_IMA_XATTR_DIGSIG); +} +Now you're calling ima_protect_xattr() twice for each setxattr. Is that safe? Is it performant? Does it matter?
The first time the call to ima_protect_xattr() prevents the security.ima from being inappropriately modified. The second time it resets the cached status flags. From a performance perspective, unnecessarily re-calcuating the file hash is worse than rechecking the security xattr string. Mimi