On Sun, Aug 14, 2022 at 2:30 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
>
> Hi Scott, Paul,
>
> On Tue, 2022-08-09 at 12:24 -0400, Paul Moore wrote:
> > On Sun, Aug 7, 2022 at 11:19 PM Guozihua (Scott) <guozihua@xxxxxxxxxx> wrote:
> > >
> > > On 2022/8/8 11:02, Guozihua (Scott) wrote:
> > > > Hi Community,
> > > >
> > > > Recently we discovered a race condition while updating SELinux policy
> > > > with IMA lsm rule enabled. Which would lead to extra files being measured.
> > > >
> > > > While SELinux policy is updated, the IDs for object types and such would
> > > > be changed, and ima_lsm_update_rules would be called.
> > > >
> > > > There are no lock applied in ima_lsm_update_rules. If user accesses a
> > > > file during this time, ima_match_rules will be matching rules based on
> > > > old SELinux au_seqno resulting in selinux_audit_rule_match returning
> > > > -ESTALE.
> > > >
> > > > However, in ima_match_rules, this error number is not handled, causing
> > > > IMA to think the LSM rule is also a match, leading to measuring extra
> > > > files.
> >
> > ...
> >
> > > > Is this the intended behavior? Or is it a good idea to add a lock for
> > > > LSM rules during update?
> >
> > I'm not the IMA expert here, but a lot of effort has been into the
> > SELinux code to enable lockless/RCU SELinux policy access and I
> > *really* don't want to have to backtrack on that.
>
> IMA initially updated it's reference to the SELinux label ids lazily.
> More recently IMA refreshes the LSM label ids based on
> register_blocking_lsm_notifier(). As a result of commit 9ad6e9cb39c6
> ("selinux: fix race between old and new sidtab"), -ESTALE is now being
> returned.
To be clear, are you seeing this only started happening after commit
9ad6e9cb39c6? If that is the case, I would suggest a retry loop
around ima_filter_rule_match() when -ESTALE is returned. I believe
that should resolve the problem, if not please let us know.
--
paul-moore.com
