From: Paul Moore <paul@paul-moore.com>
To: audit@vger.kernel.org
Cc: Andreas Steinmetz <anstein99@googlemail.com>,
John Johansen <john.johansen@canonical.com>,
Mateusz Guzik <mjguzik@gmail.com>
Subject: [PATCH v2] audit: don't take task_lock() in audit_exe_compare() code path
Date: Tue, 24 Oct 2023 12:14:33 -0400 [thread overview]
Message-ID: <20231024161432.97029-2-paul@paul-moore.com> (raw)
The get_task_exe_file() function locks the given task with task_lock()
which when used inside audit_exe_compare() can cause deadlocks on
systems that generate audit records when the task_lock() is held. We
resolve this problem with two changes: ignoring those cases where the
task being audited is not the current task, and changing our approach
to obtaining the executable file struct to not require task_lock().
With the intent of the audit exe filter being to filter on audit events
generated by processes started by the specified executable, it makes
sense that we would only want to use the exe filter on audit records
associated with the currently executing process, e.g. @current. If
we are asked to filter records using a non-@current task_struct we can
safely ignore the exe filter without negatively impacting the admin's
expectations for the exe filter.
Knowing that we only have to worry about filtering the currently
executing task in audit_exe_compare() we can do away with the
task_lock() and call get_mm_exe_file() with @current->mm directly.
Cc: <stable@vger.kernel.org>
Fixes: 5efc244346f9 ("audit: fix exe_file access in audit_exe_compare")
Reported-by: Andreas Steinmetz <anstein99@googlemail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
- v2
* dropped mmget()/mmput()
- v1
* initial revision
---
kernel/audit_watch.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 65075f1e4ac8..99da4ee8e597 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -527,11 +527,16 @@ int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark *mark)
unsigned long ino;
dev_t dev;
- exe_file = get_task_exe_file(tsk);
+ /* only do exe filtering if we are recording @current events/records */
+ if (tsk != current)
+ return 0;
+
+ exe_file = get_mm_exe_file(current->mm);
if (!exe_file)
return 0;
ino = file_inode(exe_file)->i_ino;
dev = file_inode(exe_file)->i_sb->s_dev;
fput(exe_file);
+
return audit_mark_compare(mark, ino, dev);
}
--
2.42.0
next reply other threads:[~2023-10-24 16:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-24 16:14 Paul Moore [this message]
2023-10-24 16:47 ` [PATCH v2] audit: don't take task_lock() in audit_exe_compare() code path John Johansen
2023-10-24 17:29 ` Paul Moore
2023-10-24 17:59 ` Mateusz Guzik
2023-11-14 9:29 ` Artem Savkov
2023-11-14 10:32 ` Mateusz Guzik
2023-11-14 22:29 ` Paul Moore
2023-11-14 22:32 ` Mateusz Guzik
2023-11-15 0:36 ` Paul Moore
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=20231024161432.97029-2-paul@paul-moore.com \
--to=paul@paul-moore.com \
--cc=anstein99@googlemail.com \
--cc=audit@vger.kernel.org \
--cc=john.johansen@canonical.com \
--cc=mjguzik@gmail.com \
/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