diff --git a/CMakeLists.txt b/CMakeLists.txt index 2c90405..13c0cee 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -67,6 +67,8 @@ set(AKERR_TESTS err_release_clears err_pool_exhaust err_maxval + err_refcount_double_fail + err_stacktrace_bounds ) set(AKERR_WILL_FAIL_TESTS diff --git a/include/akerror.tmpl.h b/include/akerror.tmpl.h index 8179279..78651f3 100644 --- a/include/akerror.tmpl.h +++ b/include/akerror.tmpl.h @@ -110,10 +110,32 @@ void akerr_init_errno(void); akerr_log_method("%s:%s:%d: Unable to pull an error context from the array!", __FILE__, (char *)__func__, __LINE__); \ exit(1); \ } \ - } \ - __err_context->refcount += 1; + __err_context->refcount += 1; \ + } -/* +/* + * Append a formatted line to the error's stack-trace buffer, bounded by the + * space that remains so a deep propagation chain cannot write past the end of + * stacktracebuf. snprintf reports the length it *would* have written, which on + * truncation exceeds what it actually wrote, so the cursor advance is clamped + * to the remaining space. + */ +#define AKERR_STACKTRACE_APPEND(__err_context, ...) \ + do { \ + char *__akerr_stb = (char *)__err_context->stacktracebuf; \ + size_t __akerr_used = (size_t)(__err_context->stacktracebufptr - __akerr_stb); \ + if ( __akerr_used < AKERR_MAX_ERROR_STACKTRACE_BUF_LENGTH ) { \ + size_t __akerr_rem = AKERR_MAX_ERROR_STACKTRACE_BUF_LENGTH - __akerr_used; \ + int __akerr_n = snprintf(__err_context->stacktracebufptr, __akerr_rem, __VA_ARGS__); \ + if ( __akerr_n < 0 ) { \ + __akerr_n = 0; \ + } \ + __err_context->stacktracebufptr += ((size_t)__akerr_n < __akerr_rem) \ + ? (size_t)__akerr_n : (__akerr_rem - 1); \ + } \ + } while ( 0 ) + +/* * Failure and success methods for functions that return akerr_ErrorContext * */ @@ -172,7 +194,7 @@ void akerr_init_errno(void); snprintf((char *)__err_context->function, AKERR_MAX_ERROR_FUNCTION_LENGTH, __func__); \ __err_context->lineno = __LINE__; \ snprintf((char *)__err_context->message, AKERR_MAX_ERROR_CONTEXT_STRING_LENGTH, __message, ## __VA_ARGS__); \ - __err_context->stacktracebufptr += snprintf(__err_context->stacktracebufptr, AKERR_MAX_ERROR_STACKTRACE_BUF_LENGTH, "%s:%s:%d: %d (%s) : %s\n", (char *)__err_context->fname, (char *)__err_context->function, __err_context->lineno, __err_context->status, akerr_name_for_status(__err_context->status, NULL), (__err_context->message == NULL ? "" : __err_context->message)); + AKERR_STACKTRACE_APPEND(__err_context, "%s:%s:%d: %d (%s) : %s\n", (char *)__err_context->fname, (char *)__err_context->function, __err_context->lineno, __err_context->status, akerr_name_for_status(__err_context->status, NULL), (__err_context->message == NULL ? "" : __err_context->message)); #define SUCCEED(__err_context) \ @@ -198,7 +220,7 @@ void akerr_init_errno(void); VALID(__err_context, __stmt); \ if ( __err_context != NULL ) { \ if ( __err_context->status != 0 ) { \ - __err_context->stacktracebufptr += snprintf(__err_context->stacktracebufptr, AKERR_MAX_ERROR_STACKTRACE_BUF_LENGTH, "%s:%s:%d\n", (char *)__FILE__, (char *)__func__, __LINE__); \ + AKERR_STACKTRACE_APPEND(__err_context, "%s:%s:%d\n", (char *)__FILE__, (char *)__func__, __LINE__); \ break; \ } \ } diff --git a/tests/err_refcount_double_fail.c b/tests/err_refcount_double_fail.c new file mode 100644 index 0000000..1ea759f --- /dev/null +++ b/tests/err_refcount_double_fail.c @@ -0,0 +1,51 @@ +#include "akerror.h" +#include "err_capture.h" + +/* + * Regression test for the refcount leak: ENSURE_ERROR_READY must increment + * refcount only when it *acquires* a fresh context, not on every FAIL/SUCCEED. + * A function that calls FAIL more than once on the same context and then + * propagates used to arrive at the caller with refcount 2; the caller released + * once, leaking the slot. After enough leaks the pool is exhausted and the + * library exit(1)s. + */ + +akerr_ErrorContext *validate(void) +{ + PREPARE_ERROR(e); + ATTEMPT { + FAIL(e, AKERR_VALUE, "condition 1 failed"); /* acquires the context */ + FAIL(e, AKERR_KEY, "condition 2 failed"); /* must NOT re-acquire it */ + } CLEANUP { + } PROCESS(e) { + } FINISH(e, true); /* unhandled -> propagate */ + SUCCEED_RETURN(e); +} + +/* One raise -> catch -> handle cycle; returns NULL (context released). */ +akerr_ErrorContext *one_cycle(void) +{ + PREPARE_ERROR(e); + ATTEMPT { + CATCH(e, validate()); + } CLEANUP { + } PROCESS(e) { + } HANDLE(e, AKERR_KEY) { + } HANDLE(e, AKERR_VALUE) { + } FINISH(e, false); + return e; +} + +int main(void) +{ + akerr_init(); + AKERR_CHECK(akerr_slots_in_use() == 0); + + for ( int i = 0; i < 32; i++ ) { + (void)one_cycle(); + } + + AKERR_CHECK(akerr_slots_in_use() == 0); + fprintf(stderr, "err_refcount_double_fail ok\n"); + return 0; +} diff --git a/tests/err_stacktrace_bounds.c b/tests/err_stacktrace_bounds.c new file mode 100644 index 0000000..9967f02 --- /dev/null +++ b/tests/err_stacktrace_bounds.c @@ -0,0 +1,54 @@ +#include "akerror.h" +#include "err_capture.h" +#include + +/* + * Regression test for the stack-trace buffer overflow. Each frame appended a + * line with snprintf, but passed the *full* buffer length as the size rather + * than the space remaining, and advanced the cursor by snprintf's would-be + * return value. A trace that filled the buffer therefore wrote past the end of + * stacktracebuf and ran the cursor out of bounds. + * + * We place a context in a struct with a guard region right after it, position + * the trace cursor near the end of the buffer, append one more frame, and + * require that nothing was written past the buffer and the cursor stayed in + * bounds. + */ + +static struct { + akerr_ErrorContext ctx; + unsigned char guard[512]; +} probe; + +akerr_ErrorContext *append_frame(akerr_ErrorContext *e) +{ + FAIL_RETURN(e, AKERR_VALUE, + "an error message long enough to overflow a nearly full stack trace buffer"); +} + +int main(void) +{ + akerr_init(); + + memset(&probe, 0x00, sizeof(probe)); + memset(probe.guard, 0xAA, sizeof(probe.guard)); + + akerr_ErrorContext *e = &probe.ctx; + e->refcount = 1; + /* Two bytes short of full: any real frame would overflow the old code. */ + e->stacktracebufptr = probe.ctx.stacktracebuf + + AKERR_MAX_ERROR_STACKTRACE_BUF_LENGTH - 2; + + (void)append_frame(e); + + /* Nothing may have been written past the end of stacktracebuf. */ + for ( unsigned i = 0; i < sizeof(probe.guard); i++ ) { + AKERR_CHECK(probe.guard[i] == 0xAA); + } + /* The cursor must remain within the buffer. */ + AKERR_CHECK(e->stacktracebufptr + <= probe.ctx.stacktracebuf + AKERR_MAX_ERROR_STACKTRACE_BUF_LENGTH); + + fprintf(stderr, "err_stacktrace_bounds ok\n"); + return 0; +}