Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Jun 2025 17:57:56 GMT
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 0452f5f7b37a - main - audit: move the wait from the queue length from the commit to alloc
Message-ID:  <202506181757.55IHvuO3092930@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=0452f5f7b37af81dba3953c7127385fe6f307790

commit 0452f5f7b37af81dba3953c7127385fe6f307790
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2025-06-16 16:01:12 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2025-06-18 17:57:49 +0000

    audit: move the wait from the queue length from the commit to alloc
    
    AUDIT_SYSCALL_EXIT() and indirectly audit_commit() is intended to be
    called from arbitrary top-level context.  This means that any sleepable
    locks can be owned by the caller, and which makes the sleeping in
    audit_commit() forbidden.
    
    Since we need to sleep for the record in audit_alloc() anyway, move the
    sleep for the queue limit there.  At worst, if the audit is suspended is
    disabled when we actually reach the commit location, this means that we
    lost time uselessly.
    
    PR:     287566
    Reviewed by:    markj
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D50879
---
 sys/security/audit/audit.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/sys/security/audit/audit.c b/sys/security/audit/audit.c
index a47bc9554712..05928f1c33e8 100644
--- a/sys/security/audit/audit.c
+++ b/sys/security/audit/audit.c
@@ -411,15 +411,22 @@ currecord(void)
 	return (curthread->td_ar);
 }
 
-/*
- * XXXAUDIT: Shouldn't there be logic here to sleep waiting on available
- * pre_q space, suspending the system call until there is room?
- */
 struct kaudit_record *
 audit_new(int event, struct thread *td)
 {
 	struct kaudit_record *ar;
 
+	mtx_lock(&audit_mtx);
+	audit_pre_q_len++;
+
+	/*
+	 * Constrain the number of committed audit records based on
+	 * the configurable parameter.
+	 */
+	while (audit_q_len >= audit_qctrl.aq_hiwater)
+		cv_wait(&audit_watermark_cv, &audit_mtx);
+	mtx_unlock(&audit_mtx);
+
 	/*
 	 * Note: the number of outstanding uncommitted audit records is
 	 * limited to the number of concurrent threads servicing system calls
@@ -427,11 +434,6 @@ audit_new(int event, struct thread *td)
 	 */
 	ar = uma_zalloc_arg(audit_record_zone, td, M_WAITOK);
 	ar->k_ar.ar_event = event;
-
-	mtx_lock(&audit_mtx);
-	audit_pre_q_len++;
-	mtx_unlock(&audit_mtx);
-
 	return (ar);
 }
 
@@ -565,13 +567,6 @@ audit_commit(struct kaudit_record *ar, int error, int retval)
 		return;
 	}
 
-	/*
-	 * Constrain the number of committed audit records based on the
-	 * configurable parameter.
-	 */
-	while (audit_q_len >= audit_qctrl.aq_hiwater)
-		cv_wait(&audit_watermark_cv, &audit_mtx);
-
 	TAILQ_INSERT_TAIL(&audit_q, ar, k_q);
 	audit_q_len++;
 	audit_pre_q_len--;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202506181757.55IHvuO3092930>