Re: Race conditioned discovered between ima_match_rules and ima_update_lsm_update_rules

[Mimi Zohar <zohar@linux.ibm.com>]

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@huawei.com> 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.

- How likely is it if one SELinux label is stale that other labels are
stale as well? 
- Perhaps SELinux is calling the call_blocking_lsm_notifier() too
early.  Or does SELinux need to call the notifier again after
addressing the ESTALE ids?

thanks,

Mimib