From: Mimi Zohar <zohar@linux.ibm.com>
To: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>,
linux-integrity@vger.kernel.org
Cc: sashal@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] IMA: Use delayed work to free queued keys
Date: Tue, 21 Jan 2020 21:39:26 -0500 [thread overview]
Message-ID: <1579660766.5182.7.camel@linux.ibm.com> (raw)
In-Reply-To: <20200121172829.15152-1-nramas@linux.microsoft.com>
Hi 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.
On Tue, 2020-01-21 at 09:28 -0800, Lakshmi Ramasubramanian wrote:
> A timer is used to free queued keys if a custom IMA policy is not
^to free the queued keys,
> loaded within 5 minutes after IMA subsystem is initialized. Timer
^after the IMA subsystem
^The timer handler
> 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 <nramas@linux.microsoft.com>
> Fixes: 8f5d2d06f217 ("IMA: Defined timer to free queued keys")
Thank you. This is definitely better.
Mimi
> ---
> security/integrity/ima/ima_asymmetric_keys.c | 33 ++++++++++----------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
> index 381f51708e7b..fa1bdd54a9ff 100644
> --- a/security/integrity/ima/ima_asymmetric_keys.c
> +++ b/security/integrity/ima/ima_asymmetric_keys.c
> @@ -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)
prev parent reply other threads:[~2020-01-22 2:39 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-21 17:28 [PATCH] IMA: Use delayed work to free queued keys Lakshmi Ramasubramanian
2020-01-22 2:39 ` Mimi Zohar [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1579660766.5182.7.camel@linux.ibm.com \
--to=zohar@linux.ibm.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nramas@linux.microsoft.com \
--cc=sashal@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).