Re: [PATCH v5 07/12] evm: Allow xattr/attr operations for portable signatures

[Mimi Zohar]

On Tue, 2021-05-04 at 14:28 +0000, Roberto Sassu wrote:
> > From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx]
> > Sent: Monday, May 3, 2021 2:13 AM
> > Hi Roberto,
> > 
> > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > index 2271939c5c31..2ea0f2f65ab6 100644
> > > --- a/include/linux/integrity.h
> > > +++ b/include/linux/integrity.h
> > >
> > > @@ -238,9 +241,12 @@ static enum integrity_status
> > evm_verify_hmac(struct dentry *dentry,
> > >  		break;
> > >  	}
> > >
> > > -	if (rc)
> > > -		evm_status = (rc == -ENODATA) ?
> > > -				INTEGRITY_NOXATTRS : INTEGRITY_FAIL;
> > > +	if (rc) {
> > > +		evm_status = INTEGRITY_NOXATTRS;
> > > +		if (rc != -ENODATA)
> > > +			evm_status = evm_immutable ?
> > > +				     INTEGRITY_FAIL_IMMUTABLE :
> > INTEGRITY_FAIL;
> > 
> > The original code made an exception for the -ENODATA case.   Using a
> > ternary operator made sense in that case.   Inverting the test makes
> > the code less readable.  Please use the standard "if" statement
> > instead.
> 
> Did I understand correctly that the code should be:
> 
>                 evm_status = INTEGRITY_NOXATTRS;
>                 if (rc != -ENODA
>                         evm_status = INTEGRITY_FAIL;
>                         if (evm_immutable)
>                                 evm_status = INTEGRITY_FAIL_IMMUTABLE;
>                 }
> 
 
                if (rc == -ENODATA)
                        evm_status = INTEGRITY_NOXATTRS;
                else if (evm_status == evm_immutable)
                        evm_status = INTEGRITY_FAIL_IMMUTABLE;
                else
                        evm_status = INTEGRITY_FAIL;

I think keeping it simple makes it really clear that ENODATA is an
exception.

thanks,

Mimi