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