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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html