Hello Christian Brauner,
The patch e61b135f7bfe: "integrity: implement get and set acl hook"
from Sep 22, 2022, leads to the following Smatch static checker
warning:
security/integrity/evm/evm_main.c:687 evm_inode_set_acl()
warn: duplicate check 'evm_status' (previous on line 661)
security/integrity/evm/evm_main.c
649 int evm_inode_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
650 const char *acl_name, struct posix_acl *kacl)
651 {
652 enum integrity_status evm_status;
653
654 /* Policy permits modification of the protected xattrs even though
655 * there's no HMAC key loaded
656 */
657 if (evm_initialized & EVM_ALLOW_METADATA_WRITES)
658 return 0;
659
660 evm_status = evm_verify_current_integrity(dentry);
661 if ((evm_status == INTEGRITY_PASS) ||
662 (evm_status == INTEGRITY_NOXATTRS))
663 return 0;
If evm_status == INTEGRITY_PASS then this function returns here.
664
665 /* Exception if the HMAC is not going to be calculated. */
666 if (evm_hmac_disabled() && (evm_status == INTEGRITY_NOLABEL ||
667 evm_status == INTEGRITY_UNKNOWN))
668 return 0;
669
670 /*
671 * Writing other xattrs is safe for portable signatures, as portable
672 * signatures are immutable and can never be updated.
673 */
674 if (evm_status == INTEGRITY_FAIL_IMMUTABLE)
675 return 0;
676
677 if (evm_status == INTEGRITY_PASS_IMMUTABLE &&
678 !evm_inode_set_acl_change(mnt_userns, dentry, acl_name, kacl))
679 return 0;
680
681 if (evm_status != INTEGRITY_PASS &&
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Always true.
682 evm_status != INTEGRITY_PASS_IMMUTABLE)
It feels like if evm_inode_set_acl_change() fails then it should trigger
this audit message. In other words just delete the if statement and
always call integrity_audit_msg().
683 integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
684 dentry->d_name.name, "appraise_metadata",
685 integrity_status_msg[evm_status],
686 -EPERM, 0);
--> 687 return evm_status == INTEGRITY_PASS ? 0 : -EPERM;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is harmless dead code. Just "return -EPERM;"
688 }
regards,
dan carpenter
