Don't acquire evclass_lock with a spinlock held
ClosedPublic
Actions

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

Details

Reviewers

kib
aniketp

Commits

rS337258: MFC r336205:
rS337242: MFC r336205:
rS336205: Don't acquire evclass_lock with a spinlock held

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 - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

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

Herald added a subscriber: imp. · View Herald TranscriptJul 10 2018, 11:05 PM

Harbormaster completed remote builds in B17956: Diff 45124.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.

In D16210#344023, @aniketp wrote:

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?

Don't duplicate AUDIT_SYSCALL_EXIT in exit1

Harbormaster completed remote builds in B17977: Diff 45156.Jul 11 2018, 1:15 PM

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.