* [RFC PATCH v4] audit: move the tree pruning to a dedicated thread
@ 2015-01-30 14:05 Imre Palik
2015-01-30 18:42 ` Richard Guy Briggs
2015-02-02 20:47 ` Paul Moore
0 siblings, 2 replies; 3+ messages in thread
From: Imre Palik @ 2015-01-30 14:05 UTC (permalink / raw)
To: linux-audit
Cc: Paul Moore, Eric Paris, linux-kernel, Palik, Imre, Matt Wilson
From: "Palik, Imre" <imrep@amazon.de>
When file auditing is enabled, during a low memory situation, a memory
allocation with __GFP_FS can lead to pruning the inode cache. Which can,
in turn lead to audit_tree_freeing_mark() being called. This can call
audit_schedule_prune(), that tries to fork a pruning thread, and
waits until the thread is created. But forking needs memory, and the
memory allocations there are done with __GFP_FS.
So we are waiting merrily for some __GFP_FS memory allocations to complete,
while holding some filesystem locks. This can take a while ...
This patch creates a single thread for pruning the tree from
audit_add_tree_rule(), and thus avoids the deadlock that the on-demand
thread creation can cause.
Reported-by: Matt Wilson <msw@amazon.com>
Cc: Matt Wilson <msw@amazon.com>
Signed-off-by: Imre Palik <imrep@amazon.de>
---
kernel/audit_tree.c | 88 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 60 insertions(+), 28 deletions(-)
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 2e0c974..71fd1f2 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -37,6 +37,7 @@ struct audit_chunk {
static LIST_HEAD(tree_list);
static LIST_HEAD(prune_list);
+static struct task_struct *prune_thread;
/*
* One struct chunk is attached to each inode of interest.
@@ -651,6 +652,57 @@ static int tag_mount(struct vfsmount *mnt, void *arg)
return tag_chunk(mnt->mnt_root->d_inode, arg);
}
+/*
+ * That gets run when evict_chunk() ends up needing to kill audit_tree.
+ * Runs from a separate thread.
+ */
+static int prune_tree_thread(void *unused)
+{
+ for (;;) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (list_empty(&prune_list))
+ schedule();
+ __set_current_state(TASK_RUNNING);
+
+ mutex_lock(&audit_cmd_mutex);
+ mutex_lock(&audit_filter_mutex);
+
+ while (!list_empty(&prune_list)) {
+ struct audit_tree *victim;
+
+ victim = list_entry(prune_list.next,
+ struct audit_tree, list);
+ list_del_init(&victim->list);
+
+ mutex_unlock(&audit_filter_mutex);
+
+ prune_one(victim);
+
+ mutex_lock(&audit_filter_mutex);
+ }
+
+ mutex_unlock(&audit_filter_mutex);
+ mutex_unlock(&audit_cmd_mutex);
+ }
+ return 0;
+}
+
+static int audit_launch_prune(void)
+{
+ if (prune_thread)
+ return 0;
+ prune_thread = kthread_create(prune_tree_thread, NULL,
+ "audit_prune_tree");
+ if (IS_ERR(prune_thread)) {
+ pr_err("cannot start thread audit_prune_tree");
+ prune_thread = NULL;
+ return -ENOMEM;
+ } else {
+ wake_up_process(prune_thread);
+ return 0;
+ }
+}
+
/* called with audit_filter_mutex */
int audit_add_tree_rule(struct audit_krule *rule)
{
@@ -674,6 +726,12 @@ int audit_add_tree_rule(struct audit_krule *rule)
/* do not set rule->tree yet */
mutex_unlock(&audit_filter_mutex);
+ if (unlikely(!prune_thread)) {
+ err = audit_launch_prune();
+ if (err)
+ goto Err;
+ }
+
err = kern_path(tree->pathname, 0, &path);
if (err)
goto Err;
@@ -811,36 +869,10 @@ int audit_tag_tree(char *old, char *new)
return failed;
}
-/*
- * That gets run when evict_chunk() ends up needing to kill audit_tree.
- * Runs from a separate thread.
- */
-static int prune_tree_thread(void *unused)
-{
- mutex_lock(&audit_cmd_mutex);
- mutex_lock(&audit_filter_mutex);
-
- while (!list_empty(&prune_list)) {
- struct audit_tree *victim;
-
- victim = list_entry(prune_list.next, struct audit_tree, list);
- list_del_init(&victim->list);
-
- mutex_unlock(&audit_filter_mutex);
-
- prune_one(victim);
-
- mutex_lock(&audit_filter_mutex);
- }
-
- mutex_unlock(&audit_filter_mutex);
- mutex_unlock(&audit_cmd_mutex);
- return 0;
-}
static void audit_schedule_prune(void)
{
- kthread_run(prune_tree_thread, NULL, "audit_prune_tree");
+ wake_up_process(prune_thread);
}
/*
@@ -907,9 +939,9 @@ static void evict_chunk(struct audit_chunk *chunk)
for (n = 0; n < chunk->count; n++)
list_del_init(&chunk->owners[n].list);
spin_unlock(&hash_lock);
+ mutex_unlock(&audit_filter_mutex);
if (need_prune)
audit_schedule_prune();
- mutex_unlock(&audit_filter_mutex);
}
static int audit_tree_handle_event(struct fsnotify_group *group,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH v4] audit: move the tree pruning to a dedicated thread
2015-01-30 14:05 [RFC PATCH v4] audit: move the tree pruning to a dedicated thread Imre Palik
@ 2015-01-30 18:42 ` Richard Guy Briggs
2015-02-02 20:47 ` Paul Moore
1 sibling, 0 replies; 3+ messages in thread
From: Richard Guy Briggs @ 2015-01-30 18:42 UTC (permalink / raw)
To: Imre Palik; +Cc: linux-audit, Palik, Imre, Matt Wilson, linux-kernel
On 15/01/30, Imre Palik wrote:
> From: "Palik, Imre" <imrep@amazon.de>
>
> When file auditing is enabled, during a low memory situation, a memory
> allocation with __GFP_FS can lead to pruning the inode cache. Which can,
> in turn lead to audit_tree_freeing_mark() being called. This can call
> audit_schedule_prune(), that tries to fork a pruning thread, and
> waits until the thread is created. But forking needs memory, and the
> memory allocations there are done with __GFP_FS.
>
> So we are waiting merrily for some __GFP_FS memory allocations to complete,
> while holding some filesystem locks. This can take a while ...
>
> This patch creates a single thread for pruning the tree from
> audit_add_tree_rule(), and thus avoids the deadlock that the on-demand
> thread creation can cause.
>
> Reported-by: Matt Wilson <msw@amazon.com>
> Cc: Matt Wilson <msw@amazon.com>
> Signed-off-by: Imre Palik <imrep@amazon.de>
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> kernel/audit_tree.c | 88 +++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 60 insertions(+), 28 deletions(-)
>
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index 2e0c974..71fd1f2 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -37,6 +37,7 @@ struct audit_chunk {
>
> static LIST_HEAD(tree_list);
> static LIST_HEAD(prune_list);
> +static struct task_struct *prune_thread;
>
> /*
> * One struct chunk is attached to each inode of interest.
> @@ -651,6 +652,57 @@ static int tag_mount(struct vfsmount *mnt, void *arg)
> return tag_chunk(mnt->mnt_root->d_inode, arg);
> }
>
> +/*
> + * That gets run when evict_chunk() ends up needing to kill audit_tree.
> + * Runs from a separate thread.
> + */
> +static int prune_tree_thread(void *unused)
> +{
> + for (;;) {
> + set_current_state(TASK_INTERRUPTIBLE);
> + if (list_empty(&prune_list))
> + schedule();
> + __set_current_state(TASK_RUNNING);
> +
> + mutex_lock(&audit_cmd_mutex);
> + mutex_lock(&audit_filter_mutex);
> +
> + while (!list_empty(&prune_list)) {
> + struct audit_tree *victim;
> +
> + victim = list_entry(prune_list.next,
> + struct audit_tree, list);
> + list_del_init(&victim->list);
> +
> + mutex_unlock(&audit_filter_mutex);
> +
> + prune_one(victim);
> +
> + mutex_lock(&audit_filter_mutex);
> + }
> +
> + mutex_unlock(&audit_filter_mutex);
> + mutex_unlock(&audit_cmd_mutex);
> + }
> + return 0;
> +}
> +
> +static int audit_launch_prune(void)
> +{
> + if (prune_thread)
> + return 0;
> + prune_thread = kthread_create(prune_tree_thread, NULL,
> + "audit_prune_tree");
> + if (IS_ERR(prune_thread)) {
> + pr_err("cannot start thread audit_prune_tree");
> + prune_thread = NULL;
> + return -ENOMEM;
> + } else {
> + wake_up_process(prune_thread);
> + return 0;
> + }
> +}
> +
> /* called with audit_filter_mutex */
> int audit_add_tree_rule(struct audit_krule *rule)
> {
> @@ -674,6 +726,12 @@ int audit_add_tree_rule(struct audit_krule *rule)
> /* do not set rule->tree yet */
> mutex_unlock(&audit_filter_mutex);
>
> + if (unlikely(!prune_thread)) {
> + err = audit_launch_prune();
> + if (err)
> + goto Err;
> + }
> +
> err = kern_path(tree->pathname, 0, &path);
> if (err)
> goto Err;
> @@ -811,36 +869,10 @@ int audit_tag_tree(char *old, char *new)
> return failed;
> }
>
> -/*
> - * That gets run when evict_chunk() ends up needing to kill audit_tree.
> - * Runs from a separate thread.
> - */
> -static int prune_tree_thread(void *unused)
> -{
> - mutex_lock(&audit_cmd_mutex);
> - mutex_lock(&audit_filter_mutex);
> -
> - while (!list_empty(&prune_list)) {
> - struct audit_tree *victim;
> -
> - victim = list_entry(prune_list.next, struct audit_tree, list);
> - list_del_init(&victim->list);
> -
> - mutex_unlock(&audit_filter_mutex);
> -
> - prune_one(victim);
> -
> - mutex_lock(&audit_filter_mutex);
> - }
> -
> - mutex_unlock(&audit_filter_mutex);
> - mutex_unlock(&audit_cmd_mutex);
> - return 0;
> -}
>
> static void audit_schedule_prune(void)
> {
> - kthread_run(prune_tree_thread, NULL, "audit_prune_tree");
> + wake_up_process(prune_thread);
> }
>
> /*
> @@ -907,9 +939,9 @@ static void evict_chunk(struct audit_chunk *chunk)
> for (n = 0; n < chunk->count; n++)
> list_del_init(&chunk->owners[n].list);
> spin_unlock(&hash_lock);
> + mutex_unlock(&audit_filter_mutex);
> if (need_prune)
> audit_schedule_prune();
> - mutex_unlock(&audit_filter_mutex);
> }
>
> static int audit_tree_handle_event(struct fsnotify_group *group,
> --
> 1.7.9.5
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH v4] audit: move the tree pruning to a dedicated thread
2015-01-30 14:05 [RFC PATCH v4] audit: move the tree pruning to a dedicated thread Imre Palik
2015-01-30 18:42 ` Richard Guy Briggs
@ 2015-02-02 20:47 ` Paul Moore
1 sibling, 0 replies; 3+ messages in thread
From: Paul Moore @ 2015-02-02 20:47 UTC (permalink / raw)
To: Imre Palik
Cc: linux-audit, Eric Paris, linux-kernel, Palik, Imre, Matt Wilson
On Friday, January 30, 2015 03:05:59 PM Imre Palik wrote:
> From: "Palik, Imre" <imrep@amazon.de>
>
> When file auditing is enabled, during a low memory situation, a memory
> allocation with __GFP_FS can lead to pruning the inode cache. Which can,
> in turn lead to audit_tree_freeing_mark() being called. This can call
> audit_schedule_prune(), that tries to fork a pruning thread, and
> waits until the thread is created. But forking needs memory, and the
> memory allocations there are done with __GFP_FS.
>
> So we are waiting merrily for some __GFP_FS memory allocations to complete,
> while holding some filesystem locks. This can take a while ...
>
> This patch creates a single thread for pruning the tree from
> audit_add_tree_rule(), and thus avoids the deadlock that the on-demand
> thread creation can cause.
>
> Reported-by: Matt Wilson <msw@amazon.com>
> Cc: Matt Wilson <msw@amazon.com>
> Signed-off-by: Imre Palik <imrep@amazon.de>
Thanks for your persistence on this patch, I know it can be frustrating at
times. I'm happy with this revision of the patch, but considering that we are
only one week away from the merge window opening, I'm going to queue this up
for the *next* merge window; I'll move this to the audit#next branch as soon
as the v3.20 merge window closes.
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index 2e0c974..71fd1f2 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -37,6 +37,7 @@ struct audit_chunk {
>
> static LIST_HEAD(tree_list);
> static LIST_HEAD(prune_list);
> +static struct task_struct *prune_thread;
>
> /*
> * One struct chunk is attached to each inode of interest.
> @@ -651,6 +652,57 @@ static int tag_mount(struct vfsmount *mnt, void *arg)
> return tag_chunk(mnt->mnt_root->d_inode, arg);
> }
>
> +/*
> + * That gets run when evict_chunk() ends up needing to kill audit_tree.
> + * Runs from a separate thread.
> + */
> +static int prune_tree_thread(void *unused)
> +{
> + for (;;) {
> + set_current_state(TASK_INTERRUPTIBLE);
> + if (list_empty(&prune_list))
> + schedule();
> + __set_current_state(TASK_RUNNING);
> +
> + mutex_lock(&audit_cmd_mutex);
> + mutex_lock(&audit_filter_mutex);
> +
> + while (!list_empty(&prune_list)) {
> + struct audit_tree *victim;
> +
> + victim = list_entry(prune_list.next,
> + struct audit_tree, list);
> + list_del_init(&victim->list);
> +
> + mutex_unlock(&audit_filter_mutex);
> +
> + prune_one(victim);
> +
> + mutex_lock(&audit_filter_mutex);
> + }
> +
> + mutex_unlock(&audit_filter_mutex);
> + mutex_unlock(&audit_cmd_mutex);
> + }
> + return 0;
> +}
> +
> +static int audit_launch_prune(void)
> +{
> + if (prune_thread)
> + return 0;
> + prune_thread = kthread_create(prune_tree_thread, NULL,
> + "audit_prune_tree");
> + if (IS_ERR(prune_thread)) {
> + pr_err("cannot start thread audit_prune_tree");
> + prune_thread = NULL;
> + return -ENOMEM;
> + } else {
> + wake_up_process(prune_thread);
> + return 0;
> + }
> +}
> +
> /* called with audit_filter_mutex */
> int audit_add_tree_rule(struct audit_krule *rule)
> {
> @@ -674,6 +726,12 @@ int audit_add_tree_rule(struct audit_krule *rule)
> /* do not set rule->tree yet */
> mutex_unlock(&audit_filter_mutex);
>
> + if (unlikely(!prune_thread)) {
> + err = audit_launch_prune();
> + if (err)
> + goto Err;
> + }
> +
> err = kern_path(tree->pathname, 0, &path);
> if (err)
> goto Err;
> @@ -811,36 +869,10 @@ int audit_tag_tree(char *old, char *new)
> return failed;
> }
>
> -/*
> - * That gets run when evict_chunk() ends up needing to kill audit_tree.
> - * Runs from a separate thread.
> - */
> -static int prune_tree_thread(void *unused)
> -{
> - mutex_lock(&audit_cmd_mutex);
> - mutex_lock(&audit_filter_mutex);
> -
> - while (!list_empty(&prune_list)) {
> - struct audit_tree *victim;
> -
> - victim = list_entry(prune_list.next, struct audit_tree, list);
> - list_del_init(&victim->list);
> -
> - mutex_unlock(&audit_filter_mutex);
> -
> - prune_one(victim);
> -
> - mutex_lock(&audit_filter_mutex);
> - }
> -
> - mutex_unlock(&audit_filter_mutex);
> - mutex_unlock(&audit_cmd_mutex);
> - return 0;
> -}
>
> static void audit_schedule_prune(void)
> {
> - kthread_run(prune_tree_thread, NULL, "audit_prune_tree");
> + wake_up_process(prune_thread);
> }
>
> /*
> @@ -907,9 +939,9 @@ static void evict_chunk(struct audit_chunk *chunk)
> for (n = 0; n < chunk->count; n++)
> list_del_init(&chunk->owners[n].list);
> spin_unlock(&hash_lock);
> + mutex_unlock(&audit_filter_mutex);
> if (need_prune)
> audit_schedule_prune();
> - mutex_unlock(&audit_filter_mutex);
> }
>
> static int audit_tree_handle_event(struct fsnotify_group *group,
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-02-02 20:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-30 14:05 [RFC PATCH v4] audit: move the tree pruning to a dedicated thread Imre Palik
2015-01-30 18:42 ` Richard Guy Briggs
2015-02-02 20:47 ` Paul Moore
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).