linux-kernel - Re: [PATCH v4 2/2] IMA: Call workqueue functions to measure queued keys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <152580f3-2a1f-fa33-cc25-f25747a470a5@linux.microsoft.com>
Date:   Tue, 17 Dec 2019 14:22:07 -0800
From:   Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
To:     James Bottomley <James.Bottomley@...senPartnership.com>,
        zohar@...ux.ibm.com, linux-integrity@...r.kernel.org
Cc:     eric.snowberg@...cle.com, dhowells@...hat.com,
        mathew.j.martineau@...ux.intel.com, matthewgarrett@...gle.com,
        sashal@...nel.org, jamorris@...ux.microsoft.com,
        linux-kernel@...r.kernel.org, keyrings@...r.kernel.org
Subject: Re: [PATCH v4 2/2] IMA: Call workqueue functions to measure queued
 keys

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.

> 
> Please keep in mind that ima_queue_key() returns a boolean indicating 
> whether or not the key was queued. This flag is set inside the lock - 
> please see the code snippet from ima_queue_key() below:
> 
> +    mutex_lock(&ima_keys_mutex);
> +    if (!ima_process_keys) {
> +        list_add_tail(&entry->list, &ima_keys);
> +        queued = true;
> +    }
> +    mutex_unlock(&ima_keys_mutex);
> 
> If ima_process_keys had changed from false to true, ima_queue_key() will 
> not queue the key and return false to ima_post_key_create_or_update().
> 
> Code snippet in ima_post_key_create_or_update():
> 
> +    if (!ima_process_keys)
> +        queued = ima_queue_key(keyring, payload, payload_len);
> +
> +    if (queued)
> +        return;
> 
> If the "queued" is false, ima_post_key_create_or_update() will process 
> the key immediately.
> 
>   -lakshmi

thanks,
  -lakshmi