On Thu, Sep 29, 2022 at 11:19 PM Mimi Zohar [off-list ref] wrote:

Hi Paul,

On Thu, 2022-09-29 at 15:14 -0400, Paul Moore wrote:

quoted

quoted

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index bde74fcecee3..698a8ae2fe3e 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -770,6 +770,15 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
        return result;
 }

+int ima_inode_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry,
+                     const char *acl_name, struct posix_acl *kacl)
+{
+       if (evm_revalidate_status(acl_name))
+               ima_reset_appraise_flags(d_backing_inode(dentry), 0);
+
+       return 0;
+}

While the ima_inode_set_acl() implementation above looks okay for the
remove case, I do see that the ima_inode_setxattr() function has a
call to validate_hash_algo() before calling
ima_reset_appraise_flags().  IANAIE (I Am Not An Ima Expert), but it
seems like we would still want that check in the ACL case.

Thanks, Paul.  The "ima: fix blocking of security.ima xattrs of
unsupported algorithms" patch in next-integrity branch, moves the hash
algorithm checking earlier.

Okay, thanks.  When comparing against the status quo I usually just
stick with what is in Linus' tree, but I'm happy to hear this patch is
correct.

-- 
paul-moore.com