Skip to content

Commit

Permalink
audit: don't take task_lock() in audit_exe_compare() code path
Browse files Browse the repository at this point in the history
commit 47846d5 upstream.

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: 5efc244 ("audit: fix exe_file access in audit_exe_compare")
Reported-by: Andreas Steinmetz <anstein99@googlemail.com>
Reviewed-by: John Johansen <john.johanse@canonical.com>
Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
pcmoore authored and gregkh committed Nov 28, 2023
1 parent 43cea54 commit 38968c6
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion kernel/audit_watch.c
Original file line number Diff line number Diff line change
Expand Up @@ -527,11 +527,18 @@ 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;

if (WARN_ON_ONCE(!current->mm))
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);
}

0 comments on commit 38968c6

Please sign in to comment.