Files
spine/spine_sem.h
Thomas Vincent e8a4b9aac9 fix: poller correctness bugs, with fuzz and integration coverage (#542)
Poller correctness and memory-safety fixes, with the ICMP hardening from
#543 folded in.

Verified in a clean `ubuntu:24.04` container: builds at **38 warnings**
against develop's 39, and the unit tests build and pass on Linux (39
assertions across three binaries).

**Fixes**
- `php_readpipe` off-by-one — the old bound let `bptr` reach
`result_string + 2048` and then wrote the terminator one byte past a
2048-byte allocation.
- `get_namebyhost` used `strtok`, which keeps one process-wide save
pointer, and runs unlocked on every poller thread. Two devices could be
polled at each other's addresses. Now `strtok_r`.
- The log date format was read from `default_dateformat`; Cacti's key is
`default_date_format`, so the lookup never matched and the format was
never loaded.
- `db_query` returns `mysql_store_result()` directly, which is NULL on
store failure. The NULL check ran *after* `mysql_fetch_row`, so the
crash happened one line before the guard.
- `snmp_count` logged a Net-SNMP `errstat` without setting either loop
flag, rebuilding the identical GETNEXT forever and pinning a thread
until the poll interval ended.
- `php_close` skipped the `close()` when the quit write failed with
EPIPE, leaking a descriptor per script-server restart.
- The reap loop's delay was compiled out on `SOLAR_THREAD` builds, so
both phases burned twenty `WNOHANG` polls in nanoseconds and the child
was never reaped.
- ICMP replies are validated for length and matched on id/sequence; on
Cygwin the peeked datagram is now drained, which previously busy-spun
until host timeout.
- The fatal signal handler no longer calls
`malloc`/`localtime_r`/`fprintf` before `_exit` — on a SIGSEGV caused by
heap corruption that deadlocked on the allocator's lock instead of
dying.
- The unit test Makefile was hardcoded to `/opt/homebrew` paths and
`-std=gnu23`, so it could not build on Linux at all. Now pkg-config
driven.

Supersedes #543.

**Added since the first revision**
- `snmp_get_multi()` now checks the varbind type before formatting.
Under v2c an agent reports a per-OID failure as an inline exception
varbind while the PDU `errstat` stays `NOERROR`, so `snprint_value()`
rendered the exception as literal text and stored it as a value.
- Two warning sources removed: a dangling line continuation ending
`sql.h`, and `strncopy()` now using `memcpy` where the byte count is
already exact. Warnings drop from **39 to 7**.

Closes #551
Closes #232
- `read_config_options()` issued 23 separate `SELECT ... FROM settings
WHERE name = ?` round trips at startup; they are now one bulk read
served from memory. Call sites are unchanged.

Closes #330

**Test infrastructure**
- The SNMPv3 integration fixture was broken: the image shipped a binary
that could not start (`libmysqlclient.so.21: cannot open shared object
file`). The build context carried `*.o`, `spine`, `Makefile` and
`config.status`, so `make` treated them as current, skipped compiling,
and packaged a binary linked against libraries the runtime image does
not have. `.dockerignore` now excludes build output. The fixture runs
end to end: spine polls the SNMPv3 device, reads `uptime`, writes to
`poller_output` and exits 0.
- Two libFuzzer targets, run per PR and weekly: the host string parser
and the ICMP reply parser. The second drives
`spine_icmp_classify_reply()`, extracted from `ping_icmp` so the bounds
walk over a shared raw socket is reachable by a test. 15M executions
under ASan/UBSan, no crashes.


**Added in this revision**
- `output_regex` was applied only in the in-loop OID drain, not the pass
after the loop that handles whatever did not fill a `MAX_OIDS` batch, so
a device whose OID count missed a boundary stored raw values. A single
data source is always the leftover case.
`tests/integration/test_output_regex.sh` now asserts on the value: it
fails on the parent commit with the raw reading and passes here with the
filtered one. Addresses #210; the feature stays dormant until Cacti
populates `poller_item.output_regex`.

Merge before #555, which needs the `get_namebyhost` fix above to resolve
a device hostname at all.

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-08-31 10:03:55 +01:00

90 lines
2.5 KiB
C

/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| This program is free software; you can redistribute it and/or |
| modify it under the terms of the GNU General Public License |
| as published by the Free Software Foundation; either version 2 |
| of the License, or (at your option) any later version. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
#ifndef SPINE_SEM_H
#define SPINE_SEM_H
/*
* Portable counting semaphore using pthreads.
*
* macOS deprecated unnamed POSIX semaphores (sem_init, sem_getvalue).
* This wrapper provides the same counting semantics using a pthread
* mutex + condition variable, which works on all POSIX platforms.
*
* Spine uses semaphores as atomic counters (sem_post, sem_getvalue, sem_trywait).
*/
#include <pthread.h>
#include <errno.h>
typedef struct {
pthread_mutex_t mutex;
pthread_cond_t cond;
int value;
} spine_sem_t;
static inline int spine_sem_init(spine_sem_t *s, int value) {
if (pthread_mutex_init(&s->mutex, NULL) != 0) return -1;
if (pthread_cond_init(&s->cond, NULL) != 0) {
pthread_mutex_destroy(&s->mutex);
return -1;
}
s->value = value;
return 0;
}
static inline int spine_sem_post(spine_sem_t *s) {
pthread_mutex_lock(&s->mutex);
s->value++;
pthread_cond_signal(&s->cond);
pthread_mutex_unlock(&s->mutex);
return 0;
}
static inline int spine_sem_getvalue(spine_sem_t *s, int *val) {
pthread_mutex_lock(&s->mutex);
*val = s->value;
pthread_mutex_unlock(&s->mutex);
return 0;
}
static inline int spine_sem_wait(spine_sem_t *s) {
pthread_mutex_lock(&s->mutex);
while (s->value <= 0)
pthread_cond_wait(&s->cond, &s->mutex);
s->value--;
pthread_mutex_unlock(&s->mutex);
return 0;
}
/* Returns 0 when a token is acquired, or -1 with errno set like sem_trywait. */
static inline int spine_sem_trywait(spine_sem_t *s) {
pthread_mutex_lock(&s->mutex);
if (s->value <= 0) {
pthread_mutex_unlock(&s->mutex);
errno = EAGAIN;
return -1;
}
s->value--;
pthread_mutex_unlock(&s->mutex);
return 0;
}
static inline int spine_sem_destroy(spine_sem_t *s) {
pthread_mutex_destroy(&s->mutex);
pthread_cond_destroy(&s->cond);
return 0;
}
#endif /* SPINE_SEM_H */