| Message ID | 1463742875-9836-11-git-send-email-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 20 May 2016, Andreas Gruenbacher wrote: > The return value of evm_update_evmxattr is never used. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> As I mentioned last time, the EVM code is silently ignoring errors here, and I'd prefer to see that fixed. Mimi: any comment on this?
On Wed, 2016-05-25 at 15:30 +1000, James Morris wrote: > On Fri, 20 May 2016, Andreas Gruenbacher wrote: > > > The return value of evm_update_evmxattr is never used. > > > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > > As I mentioned last time, the EVM code is silently ignoring errors here, > and I'd prefer to see that fixed. Agreed. evm_update_evmxattr() is called as a result of a "protected" xattr or some other file metadata having been modified. The two actions need to remain in sync, otherwise subsequent file access will be denied. At the point that evm_update_evmxattr() fails, there isn't much that can be done other than audit the failure. The file metadata has already been modified. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index f5f1272..8b1cef07 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -39,10 +39,9 @@ extern struct crypto_shash *hash_tfm; extern char *evm_config_xattrnames[]; int evm_init_key(void); -int evm_update_evmxattr(struct dentry *dentry, - const char *req_xattr_name, - const char *req_xattr_value, - size_t req_xattr_value_len); +void evm_update_evmxattr(struct dentry *dentry, const char *req_xattr_name, + const char *req_xattr_value, + size_t req_xattr_value_len); int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, const char *req_xattr_value, size_t req_xattr_value_len, char *digest); diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 30b6b7d0..3ac6407 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -239,24 +239,22 @@ int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, * * Expects to be called with i_mutex locked. */ -int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name, - const char *xattr_value, size_t xattr_value_len) +void evm_update_evmxattr(struct dentry *dentry, const char *xattr_name, + const char *xattr_value, size_t xattr_value_len) { struct inode *inode = d_backing_inode(dentry); struct evm_ima_xattr_data xattr_data; - int rc = 0; + int rc; rc = evm_calc_hmac(dentry, xattr_name, xattr_value, xattr_value_len, xattr_data.digest); if (rc == 0) { xattr_data.type = EVM_XATTR_HMAC; - rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM, - &xattr_data, - sizeof(xattr_data), 0); + __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM, &xattr_data, + sizeof(xattr_data), 0); } else if (rc == -ENODATA && inode->i_op->removexattr) { - rc = inode->i_op->removexattr(dentry, XATTR_NAME_EVM); + inode->i_op->removexattr(dentry, XATTR_NAME_EVM); } - return rc; } int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
The return value of evm_update_evmxattr is never used. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- security/integrity/evm/evm.h | 7 +++---- security/integrity/evm/evm_crypto.c | 14 ++++++-------- 2 files changed, 9 insertions(+), 12 deletions(-)