IMA: Use delayed work to free queued keys
Subsystems (4)
PATCH 1/1 IMA: Use delayed work to free queued keys
A timer is used to free queued keys if a custom IMA policy is not loaded within 5 minutes after IMA subsystem is initialized. Timer handler is called in interrupt context. Due to this a spinlock has to be used to synchronize access to critical section. A mutex cannot be used since a mutex can sleep.
This patch uses a delayed work to free queued keys. Since a delayed work handler is called in process context a mutex can be used.
--- security/integrity/ima/ima_asymmetric_keys.c | 33 ++++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-)
security/integrity/ima/ima_asymmetric_keys.c +16 -17
@@ -11,7 +11,7 @@#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/timer.h> +#include <linux/workqueue.h> #include <keys/asymmetric-type.h> #include "ima.h"
@@ -24,37 +24,37 @@ static bool ima_process_keys;/* * To synchronize access to the list of keys that need to be measured */ -static DEFINE_SPINLOCK (ima_keys_lock); +static DEFINE_MUTEX (ima_keys_lock);static LIST_HEAD (ima_keys);/* * If custom IMA policy is not loaded then keys queued up -* for measurement should be freed. This timer is used +* for measurement should be freed. This worker is used * for handling this scenario. */ static long ima_key_queue_timeout =300000 ;/* 5 Minutes */ -static struct timer_list ima_key_queue_timer; +static void ima_keys_handler (struct work_struct *work); +static DECLARE_DELAYED_WORK(ima_keys_delayed_work, ima_keys_handler);static bool timer_expired;/* -* This timer callback function frees keys that may still be +* This worker function frees keys that may still be * queued up in case custom IMA policy was not loaded. */ -static void ima_timer_handler (struct timer_list *timer) +static void ima_keys_handler (struct work_struct *work) { timer_expired = true;ima_process_queued_keys (); }/* -* This function sets up a timer to free queued keys in case +* This function sets up a worker to free queued keys in case * custom IMA policy was never loaded. */ void ima_init_key_queue (void ) { -timer_setup (&ima_key_queue_timer, ima_timer_handler,0 ); -mod_timer (&ima_key_queue_timer, - jiffies +msecs_to_jiffies (ima_key_queue_timeout)); +schedule_delayed_work (&ima_keys_delayed_work, +msecs_to_jiffies (ima_key_queue_timeout)); }static void ima_free_key_entry (struct ima_key_entry *entry)
@@ -103,18 +103,17 @@ static bool ima_queue_key(struct key *keyring, const void *payload, {bool queued = false;struct ima_key_entry *entry; -unsigned long flags; entry =ima_alloc_key_entry (keyring, payload, payload_len);if (!entry)return false; -spin_lock_irqsave (&ima_keys_lock, flags); +mutex_lock (&ima_keys_lock);if (!ima_process_keys) {list_add_tail (&entry->list, &ima_keys); queued = true; } -spin_unlock_irqrestore (&ima_keys_lock, flags); +mutex_unlock (&ima_keys_lock);if (!queued)ima_free_key_entry (entry);
@@ -132,7 +131,6 @@ void ima_process_queued_keys(void) {struct ima_key_entry *entry, *tmp;bool process = false; -unsigned long flags;if (ima_process_keys)return ;
@@ -143,17 +141,18 @@ void ima_process_queued_keys(void)* First one setting the ima_process_keys flag to true will * process the queued keys. */ -spin_lock_irqsave (&ima_keys_lock, flags); +mutex_lock (&ima_keys_lock);if (!ima_process_keys) { ima_process_keys = true; process = true; } -spin_unlock_irqrestore (&ima_keys_lock, flags); +mutex_unlock (&ima_keys_lock);if (!process)return ; -del_timer (&ima_key_queue_timer); +if (!timer_expired) +cancel_delayed_work_sync (&ima_keys_delayed_work);list_for_each_entry_safe (entry, tmp, &ima_keys, list) {if (!timer_expired)
Signature · 1 line
2.17.1
Mimi Zohar
maintainerHi Lackshmi,
In the subject line and the patch description please refer to the construct as "delayed workqueue" or even abbreviated as "delayed wq", even though the struct itself is named "delayed_work.
Quoted 1 line · Lakshmi Ramasubramanian
A timer is used to free queued keys if a custom IMA policy is not
^to free the queued keys,
Quoted 1 line
loaded within 5 minutes after IMA subsystem is initialized. Timer
^after the IMA subsystem
^The timer handler
Quoted 9 lines
handler is called in interrupt context. Due to this a spinlock has to be used to synchronize access to critical section. A mutex cannot be used since a mutex can sleep.
This patch uses a delayed work to free queued keys. Since a delayed work handler is called in process context a mutex can be used.
Signed-off-by Lakshmi Ramasubramanian <[email protected]>Fixes 8f5d2d06f217 ("IMA: Defined timer to free queued keys")
Thank you. This is definitely better.
Mimi
Quoted 115 lines
--- security/integrity/ima/ima_asymmetric_keys.c | 33 ++++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-)security/integrity/ima/ima_asymmetric_keys.c +16
-17@@ -11,7 +11,7 @@#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt -#include <linux/timer.h> +#include <linux/workqueue.h> #include <keys/asymmetric-type.h> #include "ima.h" @@ -24,37 +24,37 @@ static bool ima_process_keys;/* * To synchronize access to the list of keys that need to be measured */ -static DEFINE_SPINLOCK (ima_keys_lock); +static DEFINE_MUTEX (ima_keys_lock);static LIST_HEAD (ima_keys);/* * If custom IMA policy is not loaded then keys queued up -* for measurement should be freed. This timer is used +* for measurement should be freed. This worker is used * for handling this scenario. */ static long ima_key_queue_timeout =300000 ;/* 5 Minutes */ -static struct timer_list ima_key_queue_timer; +static void ima_keys_handler (struct work_struct *work); +static DECLARE_DELAYED_WORK(ima_keys_delayed_work, ima_keys_handler);static bool timer_expired;/* -* This timer callback function frees keys that may still be +* This worker function frees keys that may still be * queued up in case custom IMA policy was not loaded. */ -static void ima_timer_handler (struct timer_list *timer) +static void ima_keys_handler (struct work_struct *work) { timer_expired = true;ima_process_queued_keys (); }/* -* This function sets up a timer to free queued keys in case +* This function sets up a worker to free queued keys in case * custom IMA policy was never loaded. */ void ima_init_key_queue (void ) { -timer_setup (&ima_key_queue_timer, ima_timer_handler,0 ); -mod_timer (&ima_key_queue_timer, - jiffies +msecs_to_jiffies (ima_key_queue_timeout)); +schedule_delayed_work (&ima_keys_delayed_work, +msecs_to_jiffies (ima_key_queue_timeout)); }static void ima_free_key_entry (struct ima_key_entry *entry)@@ -103,18 +103,17 @@ static bool ima_queue_key(struct key *keyring, const void *payload, {bool queued = false;struct ima_key_entry *entry; -unsigned long flags; entry =ima_alloc_key_entry (keyring, payload, payload_len);if (!entry)return false; -spin_lock_irqsave (&ima_keys_lock, flags); +mutex_lock (&ima_keys_lock);if (!ima_process_keys) {list_add_tail (&entry->list, &ima_keys); queued = true; } -spin_unlock_irqrestore (&ima_keys_lock, flags); +mutex_unlock (&ima_keys_lock);if (!queued)ima_free_key_entry (entry);@@ -132,7 +131,6 @@ void ima_process_queued_keys(void) {struct ima_key_entry *entry, *tmp;bool process = false; -unsigned long flags;if (ima_process_keys)return ;@@ -143,17 +141,18 @@ void ima_process_queued_keys(void)* First one setting the ima_process_keys flag to true will * process the queued keys. */ -spin_lock_irqsave (&ima_keys_lock, flags); +mutex_lock (&ima_keys_lock);if (!ima_process_keys) { ima_process_keys = true; process = true; } -spin_unlock_irqrestore (&ima_keys_lock, flags); +mutex_unlock (&ima_keys_lock);if (!process)return ; -del_timer (&ima_key_queue_timer); +if (!timer_expired) +cancel_delayed_work_sync (&ima_keys_delayed_work);list_for_each_entry_safe (entry, tmp, &ima_keys, list) {if (!timer_expired)