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: <8844a360-6d1e-1435-db7c-fd7739487168@linux.microsoft.com>
Date:   Mon, 16 Dec 2019 11:20: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/15/2019 10:53 PM, James Bottomley wrote:

Hi James,

> On Sun, 2019-12-15 at 17:12 -0800, Lakshmi Ramasubramanian wrote:
>> On 12/15/2019 7:22 AM, James Bottomley wrote:
>>
>> Hi James,
>>
>>>
>>> This is the problem:
>>>
>>> if (!flag)
>>>       pre()
>>> .
>>> .
>>> .
>>> if (!flag)
>>>       post()
>>>
>>> And your pre and post function either have to both run or neither
>>> must.
>>>    However, the flag is set asynchronously, so if it gets set while
>>> another thread is running through the above code, it can change
>>> after
>>> pre is run but before post is.
>>>

> 
> That doesn't matter ... the question is, is the input assumption that
> both pre/post have to be called or neither must correct?  If so, the
> code is wrong, if not, explain why.
> 
> James
> 

I assume you are asking
"What happens if the flag changes between the check done without the 
mutex held (pre()) and the check done after the mutex is taken (post())".

If I misunderstood your question, please clarify.

"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.

"WRITER" function: ima_process_queued_keys()
********************************************
In ima_process_queued_keys() the flag is checked first without the mutex 
taken:

  => If the flag is true, either the queued keys have already been 
processed OR is in the middle of being processed. So no further action 
is required.

  => 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.

  But if the flag is still false (after the mutex is taken), then the 
queued keys are processed and the flag is set to true.

The above sequence ensures that queued keys are processed one and only 
once. Subsequent keys are always processed immediately.

To the best of my knowledge, there is no condition under which a key 
would ever be dropped or be queued up without ever getting processed.
I hope that answers your question.

If you are still not convinced, please describe a sequence of steps that 
can cause incorrect functionality.

thanks,
  -lakshmi