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: <1568ff14-316f-f2c4-84d4-7ca4c0a1936a@linux.microsoft.com>
Date:   Sun, 15 Dec 2019 17:12:02 -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 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.
> 
> James

The pre() and post() functions you have referenced above including the 
check for the flag are executed with the mutex held.

Please see Mimi's response to the v3 email. I have copied it below:

************************************
Reading the flag IS lock protected, just spread across two functions.
For performance, ima_post_key_create_or_update() checks
ima_process_keys, before calling ima_queue_key(), which takes the
mutex before checking ima_process_keys again.

As long as both the reader and writer, take the mutex before checking
the flag, the locking is fine.  The additional check, before taking
the mutex, is simply for performance.
************************************

The flag is checked with the mutex held in the "reader" - 
ima_queue_key(). The key is queued with the mutex held only if the flag 
is false.

The flag is protected in the "writer" also - ima_process_queued_keys(). 
The flag is checked with the mutex held, set to true, and queued keys 
(if any) are transferred to the temp list.

As Mimi has pointed out the additional check of the flag, before taking 
the mutex in ima_post_key_create_or_update() and in 
ima_process_queued_keys(), is for performance reason.

If the flag is true, there is no need to take the mutex to check it 
again in those functions.

thanks,
  -lakshmi