On Tue, 2019-12-17 at 14:22 -0800, Lakshmi Ramasubramanian wrote:
> Hi James,
>
> > > >
> > > > This is the problem: in the race case you may still be adding
> > > > keys to
> > > > the queue after the other thread has processed it. Those keys
> > > > won't get
> > > > processed because the flag is now false in the post check so
> > > > the
> > > > current thread won't process them either.
> > > >
> > > > James
> > > >
>
> Please let me know if you still think there is a race condition.
>
> If yes, please explain how a key would be added to the queue after
> ima_process_queued_keys() has processed queued keys.
> ima_process_keys flag will be true when queued keys have been
> processed.
This code is confusing me:
+ /*
+ * To avoid holding the mutex when processing queued keys,
+ * transfer the queued keys with the mutex held to a temp list,
+ * release the mutex, and then process the queued keys from
+ * the temp list.
+ *
+ * Since ima_process_keys is set to true, any new key will be
+ * processed immediately and not be queued.
+ */
+ INIT_LIST_HEAD(&temp_ima_keys);
+
+ mutex_lock(&ima_keys_mutex);
+
+ if (!ima_process_keys) {
+ ima_process_keys = true;
+
+ if (!list_empty(&ima_keys)) {
+ list_for_each_entry_safe(entry, tmp, &ima_keys, list)
+ list_move_tail(&entry->list, &temp_ima_keys);
+ process = true;
+ }
+ }
+
+ mutex_unlock(&ima_keys_mutex);
+
+ if (!process)
+ return;
+
+ list_for_each_entry_safe(entry, tmp, &temp_ima_keys, list) {
+ process_buffer_measurement(entry->payload, entry->payload_len,
+ entry->keyring_name, KEY_CHECK, 0,
+ entry->keyring_name);
+ list_del(&entry->list);
+ ima_free_key_entry(entry);
+ }
+}
+
The direct implication of the comment and the lock dance with the
temporary list and the processed flag is that stuff can be added to the
ima_keys list after you drop the mutex. Your explanation in the prior
couple of emails says that nothing can be added because the
ima_process_keys flag setting prevents it. If the latter is true, you
can simply drop the lock after setting the flag and rely on ima_keys
not changing to run it through process_buffer_measurement without
needing any of the intermediate list or the processed flag. If the
latter isn't true then any key added to ima_keys after the mutex is
dropped is never processed.
James
