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: <f25b7299-1530-2e43-cdf4-2208c82fc768@linux.microsoft.com>
Date:   Mon, 16 Dec 2019 13:52:26 -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

On 12/16/2019 1:37 PM, Lakshmi Ramasubramanian wrote:
> 
> On 12/16/2019 1:17 PM, James Bottomley wrote:
>> On Mon, 2019-12-16 at 11:20 -0800, Lakshmi Ramasubramanian wrote:
>>>    => If the flag is false, mutex is taken and the flag is checked
>>> again. If the flag changed from false to true between the above two
>>> tests, that means another thread had raced to call
>>> ima_process_queued_keys() and has  processed the queued keys. So
>>> again, no further action is required.
>>
>> 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 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