This does fix the bug, and with the caveat I can't vouch for ignoring non-current in the routine:
Reviewed-by: Mateusz Guzik <[email protected]> Thanks for fixing the bug. Pretty weird it took this long for it to surface. On 10/24/23, Paul Moore <[email protected]> wrote: > On Tue, Oct 24, 2023 at 2:39 PM Paul Moore <[email protected]> wrote: >> >> 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: <[email protected]> >> Fixes: 5efc244346f9 ("audit: fix exe_file access in audit_exe_compare") >> Reported-by: Andreas Steinmetz <[email protected]> >> Reviewed-by: John Johansen <[email protected]> > > John, I took the liberty of preserving your 'Reviewed-by' tag since I > felt the current->mm check was a minor addition, but if you feel > otherwise let me know and I'll remove it. > >> Signed-off-by: Paul Moore <[email protected]> >> --- >> - v3 >> * added a !current->mm check >> - v2 >> * dropped mmget()/mmput() >> - v1 >> * initial revision >> --- >> kernel/audit_watch.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) > > -- > paul-moore.com > -- Mateusz Guzik <mjguzik gmail.com>
