On 15-10-25 07:50:32, Mimi Zohar wrote:
> On Sat, 2015-10-24 at 17:06 +0300, Dmitry Kasatkin wrote:
>
> > > @@ -171,9 +172,8 @@ static int __init default_appraise_policy_setup(char
> > > *str)
> > > __setup("ima_appraise_tcb", default_appraise_policy_setup);
> > >
> > > /*
> > > - * Although the IMA policy does not change, the LSM policy can be
> > > - * reloaded, leaving the IMA LSM based rules referring to the old,
> > > - * stale LSM policy.
>
> Please don't remove this comment.
OK, i will leave it, but the beginning of the first sentence should be changed
to reflect that the IMA policy _may_ change.
> > > + * Blocking here is not legal as we hold an RCU lock.
> > > ima_update_policy()
> > > + * should make it safe to walk the list at any time.
> > >
>
> I'm not sure this comment is needed. If it is needed, this should be moved
> to
> below the function description.
>From personal experience, i like it when i see such comments, especially at
critical code section. I'll move it down below if you don't like it there.
> > > * Update the IMA LSM based rules to reflect the reloaded LSM policy.
> > > * We assume the rules still exist; and BUG_ON() if they don't.
> > > @@ -184,7 +184,6 @@ static void ima_lsm_update_rules(void)
> > > int result;
> > > int i;
> > >
> > > - mutex_lock(&ima_rules_mutex);
> > > list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
> >
> >
> > Use of "list_for_each_entry_safe" makes me having a doubt.
> >
> > "safe" versions are used when entries are removed while walk.
> >
> > If it is so, then entire RCU case is impossible?
>
> Taking the lock here was to prevent modifying the LSM rule number multiple
> times. Using the "safe" version was never needed. The worst that can happen
> here is that the LSM rule number is updated multiple times.
"safe" is a remnant from the original version of the code. I don't think it is
necessary, but didn't want to make too many changes in one go. In the worst
case the list traversal is a bit slower, which should not happen too often
anyway.
Petko
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
