Skip to content

Commit 62f7816

Browse files
committed
Fix a couple threading issues
Clarify reconfigure runs on the dispatch thread and no other thread walks the plugin conf while that happens. Mark enqueue as a single producer function so that thread analysis understands only 1 thread ever calls enqueue. Use atomics for the flush variable.
1 parent c9d2301 commit 62f7816

File tree

4 files changed

+28
-7
lines changed

4 files changed

+28
-7
lines changed

audisp/audispd.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,12 @@ static int reconfigure(void)
221221
conf_llist tmp_plugin;
222222
lnode *tpconf;
223223

224+
/*
225+
* reconfigure() executes on the dispatcher thread after the event
226+
* loop returns. No other thread walks plugin_conf while this runs,
227+
* so we can rebuild the list in place without additional locking.
228+
*/
229+
224230
if (need_queue_depth_change) {
225231
need_queue_depth_change = 0;
226232
increase_queue_depth(daemon_config.q_depth);

audisp/queue.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@
4242
* the index alone does not guarantee exclusive access to a ring buffer entry.
4343
* A compare‑exchange or other reservation mechanism (or simply a mutex) is
4444
* required to make the queue race‑free. However, auditd is the only producer
45-
* and audisp is the only consumer, so the queue is safe in practice.
45+
* and audisp is the only consumer, so the queue is safe in practice. The
46+
* enqueue() entry point is annotated with AUDIT_SINGLE_PRODUCER_FUNC to make
47+
* this contract visible to thread-safety tooling.
4648
*/
4749

4850
static volatile event_t **q;
@@ -223,7 +225,7 @@ static int do_overflow_action(struct disp_conf *config)
223225
* when processing is suspended, and
224226
* -1 on other errors
225227
*/
226-
int enqueue(event_t *e, struct disp_conf *config)
228+
AUDIT_SINGLE_PRODUCER_FUNC int enqueue(event_t *e, struct disp_conf *config)
227229
{
228230
unsigned int n, retry_cnt = 0;
229231

common/common.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@
4242
#ifndef __wur
4343
# define __wur
4444
#endif
45+
#ifndef __has_attribute
46+
# define __has_attribute(x) 0
47+
#endif
4548

4649
/* Wrapper macros for optional atomics
4750
* Note: ATOMIC_INT and ATOMIC_UNSIGNED are defined in config.h */
@@ -55,6 +58,12 @@
5558
# define AUDIT_ATOMIC_LOAD(var) (var)
5659
#endif
5760

61+
#if __has_attribute(no_thread_safety_analysis)
62+
# define AUDIT_SINGLE_PRODUCER_FUNC __attribute__((no_thread_safety_analysis))
63+
#else
64+
# define AUDIT_SINGLE_PRODUCER_FUNC
65+
#endif
66+
5867
// Used in auditd-event.c and audisp.c to size buffers for formatting
5968
#define FORMAT_BUF_LEN (MAX_AUDIT_MESSAGE_LENGTH + _POSIX_HOST_NAME_MAX)
6069

src/auditd-event.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,11 @@ static off_t log_size = 0;
9393
static pthread_t flush_thread;
9494
static pthread_mutex_t flush_lock;
9595
static pthread_cond_t do_flush;
96-
static volatile int flush;
96+
#ifdef HAVE_ATOMIC
97+
static ATOMIC_INT flush;
98+
#else
99+
static volatile ATOMIC_INT flush;
100+
#endif
97101
static auparse_state_t *au = NULL;
98102

99103
/* Local definitions */
@@ -236,14 +240,14 @@ static void *flush_thread_main(void *arg)
236240
// In the event that the logging thread requests another
237241
// flush before the first completes, this simply turns
238242
// into a loop of fsyncs.
239-
while (flush == 0) {
243+
while (AUDIT_ATOMIC_LOAD(flush) == 0) {
240244
pthread_cond_wait(&do_flush, &flush_lock);
241245
if (AUDIT_ATOMIC_LOAD(stop)) {
242246
pthread_mutex_unlock(&flush_lock);
243247
return NULL;
244248
}
245249
}
246-
flush = 0;
250+
AUDIT_ATOMIC_STORE(flush, 0);
247251
pthread_mutex_unlock(&flush_lock);
248252

249253
if (log_fd >= 0)
@@ -258,7 +262,7 @@ static void init_flush_thread(void)
258262
{
259263
pthread_mutex_init(&flush_lock, NULL);
260264
pthread_cond_init(&do_flush, NULL);
261-
flush = 0;
265+
AUDIT_ATOMIC_STORE(flush, 0);
262266
pthread_create(&flush_thread, NULL, flush_thread_main, NULL);
263267
pthread_detach(flush_thread);
264268
}
@@ -671,7 +675,7 @@ void handle_event(struct auditd_event *e)
671675
}
672676
} else {
673677
pthread_mutex_lock(&flush_lock);
674-
flush = 1;
678+
AUDIT_ATOMIC_STORE(flush, 1);
675679
pthread_cond_signal(&do_flush);
676680
pthread_mutex_unlock(
677681
&flush_lock);

0 commit comments

Comments
 (0)