lkml.org 
[lkml]   [2019]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v4 2/2] IMA: Call workqueue functions to measure queued keys
    From
    Date
    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

    \
     
     \ /
      Last update: 2019-12-18 03:02    [W:6.665 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site