On Mon, 2021-05-03 at 07:55 +0000, Roberto Sassu wrote: > > > > diff --git a/security/integrity/evm/evm_main.c > > b/security/integrity/evm/evm_main.c > > > @@ -354,6 +372,8 @@ static int evm_protect_xattr(struct dentry *dentry, > > const char *xattr_name, > > > -EPERM, 0); > > > } > > > out: > > > + if (evm_ignore_error_safe(evm_status)) > > > + return 0; > > > > I agree with the concept, but the function name doesn't provide enough > > context. Perhaps defining a function more along the lines of > > "evm_hmac_disabled()" would be more appropriate and at the same time > > self documenting. > > Since the function checks if the passed error can be ignored, > would evm_ignore_error_hmac_disabled() also be ok? The purpose of evm_protect_xattr() is to prevent allowing an invalid security.evm xattr from being re-calculated and updated, making it valid. Refer to the first line of the function description. That hasn't changed. One of the reasons for defining a new function is to avoid code duplication, but it should not come at the expense of clear and easily understood code. In this case, the reason for "ignoring" certain return codes needs to be highlighted, not hidden. (is_)evm_hmac_disabled() makes this very clear. Please update the function description to include the reason why making an exception is safe. thanks, Mimi > > > if (evm_status != INTEGRITY_PASS) > > > integrity_audit_msg(AUDIT_INTEGRITY_METADATA, > > d_backing_inode(dentry), > > > dentry->d_name.name, > > "appraise_metadata",
