Re: [PATCH v2 07/12] evm: Introduce EVM_RESET_STATUS atomic flag

On Thu, 2020-09-17 at 17:36 +0000, Roberto Sassu wrote:

quoted

quoted

diff --git a/security/integrity/evm/evm_main.c

b/security/integrity/evm/evm_main.c

quoted

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,

quoted

 		evm_status = (rc == -ENODATA) ?
 				INTEGRITY_NOXATTRS : INTEGRITY_FAIL;
 out:
-	if (iint)
+	if (iint) {
+		/*
+		 * EVM_RESET_STATUS can be cleared only by

evm_verifyxattr()

quoted

+		 * when EVM_ALLOW_METADATA_WRITES is set. This

guarantees that

quoted

+		 * IMA sees the EVM_RESET_STATUS flag set before it is

cleared.

quoted

+		 */
+		clear_bit(EVM_RESET_STATUS, &iint->atomic_flags);
 		iint->evm_status = evm_status;

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