Re: [PATCH v5 2/2] ima: measure userspace policy writes before parsing

On Mon, 2026-07-06 at 23:03 -0400, Mimi Zohar wrote:
> On Thu, 2026-07-02 at 21:04 +0200, Enrico Bravi wrote:
> 
> > diff --git a/security/integrity/ima/ima_policy.c
> > b/security/integrity/ima/ima_policy.c
> > index a65b7e4b64d6..d6d249190705 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -591,6 +593,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
> >  	switch (func) {
> >  	case KEY_CHECK:
> >  	case CRITICAL_DATA:
> > +	case POLICY_CHECK:
> >  		return ((rule->func == func) &&
> >  			ima_match_rule_data(rule, func_data, cred));
> >  	default:
> 
> Hi Enrico,
> 
> Unlike the other hooks, KEY_CHECK and CRITICAL_DATA are special cases, which
> only allow a set of keyrings or labels respectively.  POLICY_CHECK rules can
> be
> defined in terms of other file metadata (e.g. uid, gid, ...). With this
> change,
> any options specified on the rule will be not be matched.

Hi Mimi,

you're right.

> ima_match_rule_data() should only be called for buffer measurements, when
> there
> is no inode. 
> 
> +               return ((rule->func == func) && !inode &&
>                         ima_match_rule_data(rule, func_data, cred));

I was thinking that in this way, it would not trigger the measurement when
loading the policy from a file. If inode is not NULL, it directly returns false
instead of continuing. What do you thing of something like this:

        switch (func) {
+       case POLICY_CHECK:
+               if (inode)
+                       break;
+               fallthrough;
        case KEY_CHECK:
        case CRITICAL_DATA:
                return ((rule->func == func) &&
                        ima_match_rule_data(rule, func_data, cred));

> Otherwise the patch looks good.

Thank you,

Enrico

> Mimi