On Thu, 2020-09-17 at 17:36 +0000, Roberto Sassu wrote:
> > > diff --git a/security/integrity/evm/evm_main.c
> > b/security/integrity/evm/evm_main.c
> > > 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,
> > > evm_status = (rc == -ENODATA) ?
> > > INTEGRITY_NOXATTRS : INTEGRITY_FAIL;
> > > out:
> > > - if (iint)
> > > + if (iint) {
> > > + /*
> > > + * EVM_RESET_STATUS can be cleared only by
> > evm_verifyxattr()
> > > + * when EVM_ALLOW_METADATA_WRITES is set. This
> > guarantees that
> > > + * IMA sees the EVM_RESET_STATUS flag set before it is
> > cleared.
> > > + */
> > > + 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
