On Thu, Sep 29, 2022 at 11:19 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
>
> Hi Paul,
>
> On Thu, 2022-09-29 at 15:14 -0400, Paul Moore wrote:
> > > 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
