On Thu, 2020-09-17 at 17:36 +0000, Roberto Sassu wrote:
diff --git a/security/integrity/evm/evm_main.c
b/security/integrity/evm/evm_main.c
index 4e9f5e8b21d5..05be1ad3e6f3 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -221,8 +221,15 @@ static enum integrity_status
evm_verify_hmac(struct dentry *dentry,
evm_status = (rc == -ENODATA) ? INTEGRITY_NOXATTRS : INTEGRITY_FAIL;out:
- if (iint)
- if (iint) {
/** EVM_RESET_STATUS can be cleared only byevm_verifyxattr()
* when EVM_ALLOW_METADATA_WRITES is set. Thisguarantees that
* IMA sees the EVM_RESET_STATUS flag set before it iscleared.
*/clear_bit(EVM_RESET_STATUS, &iint->atomic_flags);True IMA is currently the only caller of evm_verifyxattr() in the upstreamed kernel, but it is an exported function, which may be called from elsewhere. The previous version crossed the boundary between EVM & IMA with EVM modifying the IMA flag directly. This version assumes that IMA will be the only caller. Otherwise, I like this version.
Ok, I think it is better, as you suggested, to export a new EVM function that tells if evm_reset_status() will be executed in the EVM post hooks, and to call this function from IMA. IMA would then call ima_reset_appraise_flags() also depending on the result of the new EVM function.
ima_reset_appraise_flags() should be called in a post hook in IMA. Should I introduce it?
Yes, so any callers of evm_verifyxattr() will need to implement the post hook as well. As much as possible, please limit code duplication.
The last time I looked, there didn't seem to be a locking concern, but please make sure.
thanks,
Mimi