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: <35a6c241-9a46-2657-51d1-0c04d32a9fae@linux.microsoft.com>
Date:   Mon, 16 Dec 2019 13:37:11 -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: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
> 

I am not sure how a key could get added to the queue after another 
thread has processed the queued keys.

The flag changes from false to true only once - in 
ima_process_queued_keys(). This change is done under the lock. The 
thread that makes this change will process all the queued keys.

Once the above change is done, ima_process_keys flag will never become 
false again.

Another thread that is trying to queue the key will wait on the mutex - 
in ima_queue_key(). If this thread finds the flag is true after taking 
the mutex, it will NOT queue the key.

Please see my explanation below:

"READER" functions: ima_post_key_create_or_update() and ima_queue_key()
***********************************************************************
In ima_post_key_create_or_update() the flag is checked first without the 
mutex taken:

  => If the flag is true, then there is no need to queue the key and it 
can be processed immediately.

     This condition means that either queued keys have already been 
processed OR there is another thread in the middle of processing queued 
keys. In both these conditions, the new key should NOT be queued, but 
processed immediately.

  => If the flag is false, ima_queue_key() is called. In this function, 
the mutex is taken and flag checked again.

Say, the flag changed from false to true at this point, the key will NOT 
be queued. ima_queue_key() will return false and in response 
ima_post_key_create_or_update() will process the key immediately.

But if the flag is still false, the key will be queued by 
ima_queue_key() and will be processed later.