On Wed, Oct 18, 2023 at 10:14 PM Mateusz Guzik <[email protected]> wrote: > On 10/19/23, Paul Moore <[email protected]> wrote: > > On Wed, Oct 18, 2023 at 8:22 PM Mateusz Guzik <[email protected]> wrote: > >> On 10/19/23, 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. As > >> >> we should be able to safely access current->mm we can drop the call to > >> >> get_task_exe_file() and get a reference to current->mm directly which > >> >> we can then use in a call to get_mm_exe_file() without having to take > >> >> task_lock(). > >> > >> I don't follow what's the deadlock. > >> > >> You mean they hold task lock for current and then call into > >> audit_exe_compare which takes it again? > >> > >> I would argue any calls into the routine with *any* task already > >> locked are fishy at best (I'm assuming there is an established task > >> lock ordering which may be accidentally violated). > > > > A link to the original problem report is below, but the quick summary > > is a capable() check in the do_prlimit() code path. > > > > https://lore.kernel.org/audit/[email protected] > > While get_task_exe_file working without taking the task lock would be > nice, I think the *real* bug is do_prlimit calling into > capabilities/security machinery with said lock held to begin with.
I'm not going to argue about do_prlimit() taking the task_lock(), I can't say I've thought enough about that, but regardless of what happens there I think we can fix things on the audit side as well. Thinking about it a bit more this morning, I think we can safely ignore the non-@current case in audit_exe_compare() as the whole point of the audit exe filter is to record the actions of processes instantiated from that executable file; if the @current task is not being logged/filtered, we shouldn't have to worry about the exe filter. I'll send out a v2 of the patch later with those changes, but like I said at the top, please don't let that stop you making improvements elsewhere. -- paul-moore.com
