Page MenuHomeFreeBSD

Don't acquire evclass_lock with a spinlock held
ClosedPublic

Authored by asomers on Jul 10 2018, 11:05 PM.

Details

Summary

When the "pc" audit class is enabled and auditd is running, witness will
panic during thread exit because au_event_class tries to lock an rwlock
while holding a spinlock acquired upstack by thread_exit.

To fix this, move AUDIT_SYSCALL_EXIT futher upstack, before the spinlock is
acquired. Of thread_exit's 16 callers, it's only necessary to call
AUDIT_SYSCALL_EXIT from two, exit1 (for exiting processes) and kern_thr_exit
(for exiting threads). The other callers are all kernel threads, which
needen't call AUDIT_SYSCALL_EXIT because since they can't make syscalls
there will be nothing to audit.

PR: 228444

Test Plan

The audit(4) test suite

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

asomers created this revision.Jul 10 2018, 11:05 PM

@aniketp are there any exit-related audit events that you haven't tested yet? It would be good to check that this change doesn't break them.

aniketp added inline comments.Jul 11 2018, 6:57 AM
sys/kern/kern_exit.c
311 ↗(On Diff #45124)

@asomers Don't we already have AUDIT_SYSCALL_EXIT(0, td) here in the initial part of exit1()?

Other than the (possible?) repetition of AUDIT_SYSCALL_EXIT in kern_exit.c. The issue with exa(1) seems to have gone away. Also, the regression tests for audit of _exit(2) in D16099 don't fail on introducing this change.

Other than the (possible?) repetition of AUDIT_SYSCALL_EXIT in kern_exit.c. The issue with exa(1) seems to have gone away. Also, the regression tests for audit of _exit(2) in D16099 don't fail on introducing this change.

Yeah, I didn't notice the other AUDIT_SYSCALL_EXIT. I guess the second one can be removed. And what about the _exit(2) test? How was it previously failing?

asomers updated this revision to Diff 45156.Jul 11 2018, 1:15 PM

Don't duplicate AUDIT_SYSCALL_EXIT in exit1

Oh, no. _exit(2) wasn't failing previously. In fact, I hadn't even created its test. I apparently overlooked it just because process-control has a _lot_ of syscalls.

sys/kern/kern_exit.c
648 ↗(On Diff #45156)

Btw, is this space intentional? It wasn't there before.

asomers updated this revision to Diff 45164.Jul 11 2018, 2:31 PM

Remove errant newline

aniketp accepted this revision.Jul 11 2018, 2:33 PM
This revision is now accepted and ready to land.Jul 11 2018, 2:33 PM
kib accepted this revision.Jul 11 2018, 7:00 PM
This revision was automatically updated to reflect the committed changes.