Re: [PATCH] ima: Handle -ESTALE returned by ima_filter_rule_match()

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

On Tue, 2022-08-30 at 16:41 +0800, Guozihua (Scott) wrote:
> On 2022/8/30 9:20, Mimi Zohar wrote:
> > On Sat, 2022-08-27 at 17:57 +0800, Guozihua (Scott) wrote:
> >> On 2022/8/25 21:02, Mimi Zohar wrote:
> >>> On Wed, 2022-08-24 at 09:56 +0800, Guozihua (Scott) wrote:
> >>>> On 2022/8/24 9:26, Mimi Zohar wrote:
> >>>>> On Tue, 2022-08-23 at 21:28 +0800, Guozihua (Scott) wrote:
> >>>>>> On 2022/8/23 21:21, Mimi Zohar wrote:
> >>>>>>> On Tue, 2022-08-23 at 16:12 +0800, Guozihua (Scott) wrote:
> >>>>>>>>> The question is whether we're waiting for the SELinux policy to change
> >>>>>>>>> from ESTALE or whether it is the number of SELinux based IMA policy
> >>>>>>>>> rules or some combination of the two.  Retrying three times seems to be
> >>>>>>>>> random.  If SELinux waited for ESTALE to change, then it would only be
> >>>>>>>>> dependent on the time it took to update the SELinux based IMA policy
> >>>>>>>>> rules.
> >>>>>>>>
> >>>>>>>> We are waiting for ima_lsm_update_rules() to finish re-initializing all
> >>>>>>>> the LSM based rules.
> >>>>>>>
> >>>>>>> Fine.  Hopefully retrying a maximum of 3 times is sufficient.
> >>>>>>>
> >>>>>> Well, at least this should greatly reduce the chance of this issue from
> >>>>>> happening.
> >>>>>
> >>>>> Agreed
> >>>>>
> >>>>>> This would be the best we I can think of without locking and
> >>>>>> busy waiting. Maybe we can also add delays before we retry. Maybe you
> >>>>>> got any other thought in mind?
> >>>>>
> >>>>> Another option would be to re-introduce the equivalent of the "lazy"
> >>>>> LSM update on -ESTALE, but without updating the policy rule, as the
> >>>>> notifier callback will eventually get to it.
> >>>>>
> >>>>
> >>>> For this to happen we would need a way to tell when we are able to
> >>>> continue with the retry though.
> >>>
> >>> Previously with the lazy update, on failure security_filter_rule_init()
> >>> was called before the retry.  To avoid locking or detecting when to
> >>> continue, another option would be to call to
> >>> security_filter_rule_init() with a local copy of the rule.  The retry
> >>> would be based on a local copy of the rule.
> >>>
> >>> Eventually the registered callback will complete, so we don't need to
> >>> be concerned about updating the actual rules.
> >>
> >> Is it possible to cause race condition though? With this, the notifier
> >> path seems to be unnecessary.
> > 
> > I don't see how there would be a race condition.  The notifier callback
> > is the normal method of updating the policy rules.  Hopefully -ESTALE
> > isn't something that happens frequently.
> 
> The notifier callback uses RCU to update rules, I think we should mimic 
> that behavior if we are to update individual rules in the matching logic.

If the callback update hasn't completed causing an -ESTALE, the
fallback is to directly query the LSM for a single IMA policy rule. 
Please keep it simple.

-- 
thanks,

Mimi