IMA: Defined timer to free queued keys
Subsystems (5)
PATCH 1/1 IMA: Defined timer to free queued keys
keys queued for measurement should still be processed even if a custom IMA policy was not loaded. Otherwise, the keys will remain queued forever consuming kernel memory.
This patch defines a timer to handle the above scenario. The timer is setup to expire 5 minutes after IMA initialization is completed.
If a custom IMA policy is loaded before the timer expires, the timer is removed and any queued keys are processed for measurement. But if a custom policy was not loaded, on timer expiration any queued keys are just freed.
--- security/integrity/ima/ima.h | 2 + security/integrity/ima/ima_asymmetric_keys.c | 46 ++++++++++++++++++-- security/integrity/ima/ima_init.c | 8 +++- 3 files changed, 52 insertions(+), 4 deletions(-)
security/integrity/ima/ima.h +2 -0
@@ -216,8 +216,10 @@ struct ima_key_entry { char *keyring_name; };void ima_process_queued_keys (void ); +void ima_init_key_queue (void );#else static inline void ima_process_queued_keys (void ) {} +static inline void ima_init_key_queue (void ) {}#endif /* CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE */ /* LIM API function definitions */
security/integrity/ima/ima_asymmetric_keys.c +43 -3
71 lines not on this page · raw message
security/integrity/ima/ima_init.c +7 -1
@@ -131,5 +131,11 @@ int __init ima_init(void)ima_init_policy (); -return ima_fs_init (); + rc =ima_fs_init (); +if (rc !=0 ) +return rc; + +ima_init_key_queue (); + +return rc; }
Signature · 1 line
2.17.1
Mimi Zohar
maintainerQuoted 7 lines · Lakshmi Ramasubramanian
keys queued for measurement should still be processed even if a custom IMA policy was not loaded. Otherwise, the keys will remain queued forever consuming kernel memory.
This patch defines a timer to handle the above scenario. The timer is setup to expire 5 minutes after IMA initialization is completed.
Quoted 6 lines
If a custom IMA policy is loaded before the timer expires, the timer is removed and any queued keys are processed for measurement. But if a custom policy was not loaded, on timer expiration any queued keys are just freed.
Signed-off-by Lakshmi Ramasubramanian <[email protected]>
This patch description, and the comments below, read as though the queued keys are measured based on policy. The queued keys should be removed, not "processed". Please rewrite this patch description and update the comments below.
Quoted 39 lines
--- security/integrity/ima/ima.h | 2 + security/integrity/ima/ima_asymmetric_keys.c | 46 ++++++++++++++++++-- security/integrity/ima/ima_init.c | 8 +++- 3 files changed, 52 insertions(+), 4 deletions(-)security/integrity/ima/ima.h +2
-0@@ -216,8 +216,10 @@ struct ima_key_entry { char *keyring_name; };void ima_process_queued_keys (void ); +void ima_init_key_queue (void );#else static inline void ima_process_queued_keys (void ) {} +static inline void ima_init_key_queue (void ) {}#endif /* CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE */ /* LIM API function definitions */ security/integrity/ima/ima_asymmetric_keys.c +3
-0@@ -11,6 +11,7 @@#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt +#include <linux/timer.h> #include <keys/asymmetric-type.h> #include "ima.h" @@ -26,6 +27,34 @@ static bool ima_process_keys;static DEFINE_MUTEX (ima_keys_mutex);static LIST_HEAD (ima_keys); +/* +* If custom IMA policy is not loaded then keys queued up
security/integrity/ima/ima_asymmetric_keys.c static bool ima_process_keys; · line 41
Quoted 10 lines
security/integrity/ima/ima_asymmetric_keys.c +10
-0+* for measurement should be freed. This timer is used +* for handling this scenario. +*/ +static long ima_key_queue_timeout =300000 ;/* 5 Minutes */ +static struct timer_list ima_key_queue_timer; + +/* +* This timer callback function processes keys that may still be +* queued up in case custom IMA policy was not loaded. +*/
"processes keys" implies measuring keys based on policy. Please update this and the comment below.
Quoted 28 lines
security/integrity/ima/ima_asymmetric_keys.c +17
-0+static void ima_timer_handler (struct timer_list *timer) +{ +ima_process_queued_keys (); +} + +/* +* This function sets up a timer to process 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)); +} +static void ima_free_key_entry (struct ima_key_entry *entry) {if (entry) {@@ -100,6 +129,7 @@ void ima_process_queued_keys(void) {struct ima_key_entry *entry, *tmp;bool process = false; +int timer_removed;if (ima_process_keys)return ;
security/integrity/ima/ima_asymmetric_keys.c void ima_process_queued_keys(void) · line 158
Quoted 9 lines
security/integrity/ima/ima_asymmetric_keys.c +6
-0@@ -120,10 +150,20 @@ void ima_process_queued_keys(void)if (!process)return ; +/* +* Timer is no longer needed. If the timer was removed +* (i.e., the timer had not expired) the queued keys are +* processed for measurement. Else, the keys are just freed. +*/ + timer_removed =del_timer (&ima_key_queue_timer);
Interesting, but wouldn't it be simpler to define a static variable named timer_expired and set it in ima_timer_handler()? With that change, the code would be easier to read and this comment would be superfluous.
Mimi
Quoted 14 lines
security/integrity/ima/ima_asymmetric_keys.c +7
-3+list_for_each_entry_safe (entry, tmp, &ima_keys, list) { -process_buffer_measurement (entry->payload, entry->payload_len, - entry->keyring_name,KEY_CHECK ,0 , - entry->keyring_name); +if (timer_removed) +process_buffer_measurement (entry->payload, + entry->payload_len, + entry->keyring_name, +KEY_CHECK ,0 , + entry->keyring_name);list_del (&entry->list);ima_free_key_entry (entry); }