Index: head/sys/security/audit/audit.h =================================================================== --- head/sys/security/audit/audit.h +++ head/sys/security/audit/audit.h @@ -2,7 +2,7 @@ * SPDX-License-Identifier: BSD-3-Clause * * Copyright (c) 1999-2005 Apple Inc. - * Copyright (c) 2016-2017 Robert N. M. Watson + * Copyright (c) 2016-2018 Robert N. M. Watson * All rights reserved. * * This software was developed by BAE Systems, the University of Cambridge @@ -55,14 +55,23 @@ #include /* - * Audit subsystem condition flags. The audit_enabled flag is set and + * Audit subsystem condition flags. The audit_trail_enabled flag is set and * removed automatically as a result of configuring log files, and can be * observed but should not be directly manipulated. The audit suspension * flag permits audit to be temporarily disabled without reconfiguring the * audit target. + * + * As DTrace can also request system-call auditing, a further + * audit_syscalls_enabled flag tracks whether newly entering system calls + * should be considered for auditing or not. + * + * XXXRW: Move trail flags to audit_private.h, as they no longer need to be + * visible outside the audit code...? */ -extern int audit_enabled; -extern int audit_suspended; +extern u_int audit_dtrace_enabled; +extern int audit_trail_enabled; +extern int audit_trail_suspended; +extern int audit_syscalls_enabled; void audit_syscall_enter(unsigned short code, struct thread *td); void audit_syscall_exit(int error, struct thread *td); @@ -139,7 +148,7 @@ /* * Define macros to wrap the audit_arg_* calls by checking the global - * audit_enabled flag before performing the actual call. + * audit_syscalls_enabled flag before performing the actual call. */ #define AUDITING_TD(td) ((td)->td_pflags & TDP_AUDITREC) @@ -369,7 +378,7 @@ } while (0) #define AUDIT_SYSCALL_ENTER(code, td) do { \ - if (audit_enabled) { \ + if (audit_syscalls_enabled) { \ audit_syscall_enter(code, td); \ } \ } while (0) @@ -377,7 +386,7 @@ /* * Wrap the audit_syscall_exit() function so that it is called only when * we have a audit record on the thread. Audit records can persist after - * auditing is disabled, so we don't just check audit_enabled here. + * auditing is disabled, so we don't just check audit_syscalls_enabled here. */ #define AUDIT_SYSCALL_EXIT(error, td) do { \ if (td->td_pflags & TDP_AUDITREC) \ Index: head/sys/security/audit/audit.c =================================================================== --- head/sys/security/audit/audit.c +++ head/sys/security/audit/audit.c @@ -2,7 +2,7 @@ * SPDX-License-Identifier: BSD-3-Clause * * Copyright (c) 1999-2005 Apple Inc. - * Copyright (c) 2006-2007, 2016-2017 Robert N. M. Watson + * Copyright (c) 2006-2007, 2016-2018 Robert N. M. Watson * All rights reserved. * * Portions of this software were developed by BAE Systems, the University of @@ -98,8 +98,12 @@ * * Define the audit control flags. */ -int __read_frequently audit_enabled; -int audit_suspended; +int audit_trail_enabled; +int audit_trail_suspended; +#ifdef KDTRACE_HOOKS +u_int audit_dtrace_enabled; +#endif +int __read_frequently audit_syscalls_enabled; /* * Flags controlling behavior in low storage situations. Should we panic if @@ -198,7 +202,34 @@ #define KINFO_RUNLOCK() rw_runlock(&audit_kinfo_lock) #define KINFO_WUNLOCK() rw_wunlock(&audit_kinfo_lock) +/* + * Check various policies to see if we should enable system-call audit hooks. + * Note that despite the mutex being held, we want to assign a value exactly + * once, as checks of the flag are performed lock-free for performance + * reasons. The mutex is used to get a consistent snapshot of policy state -- + * e.g., safely accessing the two audit_trail flags. + */ void +audit_syscalls_enabled_update(void) +{ + + mtx_lock(&audit_mtx); +#ifdef KDTRACE_HOOKS + if (audit_dtrace_enabled) + audit_syscalls_enabled = 1; + else { +#endif + if (audit_trail_enabled && !audit_trail_suspended) + audit_syscalls_enabled = 1; + else + audit_syscalls_enabled = 0; +#ifdef KDTRACE_HOOKS + } +#endif + mtx_unlock(&audit_mtx); +} + +void audit_set_kinfo(struct auditinfo_addr *ak) { @@ -303,8 +334,9 @@ audit_init(void) { - audit_enabled = 0; - audit_suspended = 0; + audit_trail_enabled = 0; + audit_trail_suspended = 0; + audit_syscalls_enabled = 0; audit_panic_on_write_fail = 0; audit_fail_stop = 0; audit_in_failure = 0; @@ -337,6 +369,9 @@ sizeof(struct kaudit_record), audit_record_ctor, audit_record_dtor, NULL, NULL, UMA_ALIGN_PTR, 0); + /* First initialisation of audit_syscalls_enabled. */ + audit_syscalls_enabled_update(); + /* Initialize the BSM audit subsystem. */ kau_init(); @@ -378,10 +413,6 @@ } /* - * XXXAUDIT: There are a number of races present in the code below due to - * release and re-grab of the mutex. The code should be revised to become - * slightly less racy. - * * XXXAUDIT: Shouldn't there be logic here to sleep waiting on available * pre_q space, suspending the system call until there is room? */ @@ -389,14 +420,7 @@ audit_new(int event, struct thread *td) { struct kaudit_record *ar; - int no_record; - mtx_lock(&audit_mtx); - no_record = (audit_suspended || !audit_enabled); - mtx_unlock(&audit_mtx); - if (no_record) - return (NULL); - /* * Note: the number of outstanding uncommitted audit records is * limited to the number of concurrent threads servicing system calls @@ -529,9 +553,13 @@ /* * Note: it could be that some records initiated while audit was * enabled should still be committed? + * + * NB: The check here is not for audit_syscalls because any + * DTrace-related obligations have been fulfilled above -- we're just + * down to the trail and pipes now. */ mtx_lock(&audit_mtx); - if (audit_suspended || !audit_enabled) { + if (audit_trail_suspended || !audit_trail_enabled) { audit_pre_q_len--; mtx_unlock(&audit_mtx); audit_free(ar); @@ -557,6 +585,10 @@ * responsible for deciding whether or not to audit the call (preselection), * and if so, allocating a per-thread audit record. audit_new() will fill in * basic thread/credential properties. + * + * This function will be entered only if audit_syscalls_enabled was set in the + * macro wrapper for this function. It could be cleared by the time this + * function runs, but that is an acceptable race. */ void audit_syscall_enter(unsigned short code, struct thread *td) Index: head/sys/security/audit/audit_dtrace.c =================================================================== --- head/sys/security/audit/audit_dtrace.c +++ head/sys/security/audit/audit_dtrace.c @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2016 Robert N. M. Watson + * Copyright (c) 2016, 2018 Robert N. M. Watson * All rights reserved. * * This software was developed by BAE Systems, the University of Cambridge @@ -147,8 +147,12 @@ * maintain a global flag tracking whether any dtaudit probes are enabled. If * not, don't bother doing all that work whenever potential queries about * events turn up during preselection or commit. + * + * NB: We used to maintain our own variable in dtaudit, but now use the + * centralized audit_dtrace_enabled variable imported from the audit code. + * + * static uint_t dtaudit_probes_enabled; */ -static uint_t dtaudit_probes_enabled; /* * Check dtaudit policy for the event to see whether this is an event we would @@ -179,7 +183,7 @@ * NB: Lockless reads here may return a slightly stale value; this is * considered better than acquiring a lock, however. */ - if (!dtaudit_probes_enabled) + if (!audit_dtrace_enabled) return (NULL); ene = au_evnamemap_lookup(event); if (ene == NULL) @@ -457,7 +461,8 @@ ene->ene_commit_probe_enabled = 1; else ene->ene_bsm_probe_enabled = 1; - refcount_acquire(&dtaudit_probes_enabled); + refcount_acquire(&audit_dtrace_enabled); + audit_syscalls_enabled_update(); } static void @@ -474,7 +479,8 @@ ene->ene_commit_probe_enabled = 0; else ene->ene_bsm_probe_enabled = 0; - (void)refcount_release(&dtaudit_probes_enabled); + (void)refcount_release(&audit_dtrace_enabled); + audit_syscalls_enabled_update(); } static void Index: head/sys/security/audit/audit_private.h =================================================================== --- head/sys/security/audit/audit_private.h +++ head/sys/security/audit/audit_private.h @@ -2,7 +2,7 @@ * SPDX-License-Identifier: BSD-3-Clause * * Copyright (c) 1999-2009 Apple Inc. - * Copyright (c) 2016-2017 Robert N. M. Watson + * Copyright (c) 2016, 2018 Robert N. M. Watson * All rights reserved. * * Portions of this software were developed by BAE Systems, the University of @@ -342,6 +342,13 @@ void audit_commit(struct kaudit_record *ar, int error, int retval); struct kaudit_record *audit_new(int event, struct thread *td); + +/* + * Function to update the audit_syscalls_enabled flag, whose value is affected + * by configuration of the audit trail/pipe mechanism and DTrace. Call this + * function when any of the inputs to that policy change. + */ +void audit_syscalls_enabled_update(void); /* * Functions relating to the conversion of internal kernel audit records to Index: head/sys/security/audit/audit_syscalls.c =================================================================== --- head/sys/security/audit/audit_syscalls.c +++ head/sys/security/audit/audit_syscalls.c @@ -2,7 +2,7 @@ * SPDX-License-Identifier: BSD-3-Clause * * Copyright (c) 1999-2009 Apple Inc. - * Copyright (c) 2016 Robert N. M. Watson + * Copyright (c) 2016, 2018 Robert N. M. Watson * All rights reserved. * * Portions of this software were developed by BAE Systems, the University of @@ -368,7 +368,7 @@ case A_OLDGETCOND: case A_GETCOND: if (uap->length == sizeof(udata.au_cond64)) { - if (audit_enabled && !audit_suspended) + if (audit_trail_enabled && !audit_trail_suspended) udata.au_cond64 = AUC_AUDITING; else udata.au_cond64 = AUC_NOAUDIT; @@ -376,7 +376,7 @@ } if (uap->length != sizeof(udata.au_cond)) return (EINVAL); - if (audit_enabled && !audit_suspended) + if (audit_trail_enabled && !audit_trail_suspended) udata.au_cond = AUC_AUDITING; else udata.au_cond = AUC_NOAUDIT; @@ -386,25 +386,27 @@ case A_SETCOND: if (uap->length == sizeof(udata.au_cond64)) { if (udata.au_cond64 == AUC_NOAUDIT) - audit_suspended = 1; + audit_trail_suspended = 1; if (udata.au_cond64 == AUC_AUDITING) - audit_suspended = 0; + audit_trail_suspended = 0; if (udata.au_cond64 == AUC_DISABLED) { - audit_suspended = 1; + audit_trail_suspended = 1; audit_shutdown(NULL, 0); } + audit_syscalls_enabled_update(); break; } if (uap->length != sizeof(udata.au_cond)) return (EINVAL); if (udata.au_cond == AUC_NOAUDIT) - audit_suspended = 1; + audit_trail_suspended = 1; if (udata.au_cond == AUC_AUDITING) - audit_suspended = 0; + audit_trail_suspended = 0; if (udata.au_cond == AUC_DISABLED) { - audit_suspended = 1; + audit_trail_suspended = 1; audit_shutdown(NULL, 0); } + audit_syscalls_enabled_update(); break; case A_GETCLASS: @@ -826,10 +828,11 @@ crhold(cred); /* - * XXXAUDIT: Should audit_suspended actually be cleared by + * XXXAUDIT: Should audit_trail_suspended actually be cleared by * audit_worker? */ - audit_suspended = 0; + audit_trail_suspended = 0; + audit_syscalls_enabled_update(); audit_rotate_vnode(cred, vp); Index: head/sys/security/audit/audit_worker.c =================================================================== --- head/sys/security/audit/audit_worker.c +++ head/sys/security/audit/audit_worker.c @@ -2,7 +2,7 @@ * SPDX-License-Identifier: BSD-3-Clause * * Copyright (c) 1999-2008 Apple Inc. - * Copyright (c) 2006-2008, 2016 Robert N. M. Watson + * Copyright (c) 2006-2008, 2016, 2018 Robert N. M. Watson * All rights reserved. * * Portions of this software were developed by BAE Systems, the University of @@ -305,7 +305,8 @@ "Audit log space exhausted and fail-stop set."); } (void)audit_send_trigger(AUDIT_TRIGGER_NO_SPACE); - audit_suspended = 1; + audit_trail_suspended = 1; + audit_syscalls_enabled_update(); /* FALLTHROUGH */ fail: @@ -518,7 +519,8 @@ audit_vp = vp; audit_size = vattr.va_size; audit_file_rotate_wait = 0; - audit_enabled = (audit_vp != NULL); + audit_trail_enabled = (audit_vp != NULL); + audit_syscalls_enabled_update(); AUDIT_WORKER_UNLOCK(); /*