On Fri, 2026-09-18 at 16:11 -0500, Frederick Lawler wrote:
> Hi Enrico,
>
> On Thu, Sep 17, 2026 at 10:58:22PM +0000, Enrico Bravi wrote:
> > Hi Frederick,
> >
> > On Wed, 2026-09-16 at 16:36 -0500, Frederick Lawler wrote:
> > > integrity_audit_msg() hides error codes by wrapping
> > > integrity_audit_message() which obfuscates the underlying reason for the
> > > failure.
> > >
> > > Update integrity_audit_msg() call sites
> >
> > here it could be mentioned that also integrity_audit_msg() itself is
> > updated.
>
> Good point. I can see that I may have not been clear about that.
>
> > > diff --git a/security/integrity/ima/ima_fs.c
> > > b/security/integrity/ima/ima_fs.c
> > > index
> > > 2a0bca5543161912abe2a0236c9fcae7055e62bc..6d2ef44b21f8d40b4981a6a441cf7c3d
> > > 69f9
> > > 4dee 100644
> > > --- a/security/integrity/ima/ima_fs.c
> > > +++ b/security/integrity/ima/ima_fs.c
> > > @@ -595,7 +595,7 @@ static ssize_t ima_write_policy(struct file *file,
> > > const
> > > char __user *buf,
> > > pr_err("signed policy file (specified as an absolute pathname)
> > > required\n");
> > > integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
> > > "policy_update", "signed policy required",
> > > - 1, 0);
> > > + 1, 0, -EINVAL);
> >
> > Here it could be put -EACCES as errno, being the value returned for this
> > case.
>
> Sashiko said the same. I kept going back and forth on this one because
> of what the message actually says. EACCES is the result, but the string
> itself + pr_error() implies that it's an input error. Similar to
> ima_release_policy() below, I should probably keep this zero, and
> follow up.
Hi Frederick,
AFAIU these messages justify why the policy loading has been denied. The point
seems to be that the input policy, even if it is valid, cannot be verified in
that form, because the signature is missing. The user can write the policy
directly on the securityfs file, but is not allowed to when a signed policy
is required. For this reason,
-EACCES could be better, because it is referring to an action permitted by the
system, but not allowed by the current policy.
Regards,
Enrico
> > > result = -EACCES;
> > > } else {
> > > ima_measure_raw_policy(data, datalen);
> > > @@ -745,7 +745,7 @@ static int ima_release_policy(struct inode *inode,
> > > struct
> > > file *file)
> > >
> > > pr_info("policy update %s\n", cause);
> > > integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
> > > - "policy_update", cause, !valid_policy, 0);
> > > + "policy_update", cause, !valid_policy, 0, 0);
> >
> > Here maybe it could be put a conditional errno, based on the validity of the
> > policy, instead of hardcoding a zero.
>
> I can see that making sense to throw a -EINVAL on it. The function
> is designed to always succeed, but it might be worth pulling out the
> error code from ima_check_policy() for this case.
>
> Best,
> Fred
