Fix refcount leak and stack-trace buffer overflow
Two memory-safety bugs in the macro core: 1. Refcount leak. ENSURE_ERROR_READY incremented refcount on every FAIL/SUCCEED rather than only when it acquired a fresh context from the pool. A function that FAILed a context more than once and then propagated arrived at its caller with refcount 2; the caller released once, leaking the slot. After AKERR_MAX_ARRAY_ERROR leaks the pool is exhausted and the library exit(1)s. Move the increment inside the acquisition branch. 2. Stack-trace overflow. Each appended frame passed the full buffer length to snprintf instead of the space remaining, and advanced the cursor by snprintf's would-be return value, so a trace that filled the buffer wrote past the end of stacktracebuf and ran the cursor out of bounds. Add AKERR_STACKTRACE_APPEND, which bounds the write to the remaining space and clamps the cursor advance. Regression tests err_refcount_double_fail and err_stacktrace_bounds fail against the old code (verified) and pass now. Full suite: 21/21, no warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -67,6 +67,8 @@ set(AKERR_TESTS
|
|||||||
err_release_clears
|
err_release_clears
|
||||||
err_pool_exhaust
|
err_pool_exhaust
|
||||||
err_maxval
|
err_maxval
|
||||||
|
err_refcount_double_fail
|
||||||
|
err_stacktrace_bounds
|
||||||
)
|
)
|
||||||
|
|
||||||
set(AKERR_WILL_FAIL_TESTS
|
set(AKERR_WILL_FAIL_TESTS
|
||||||
|
|||||||
@@ -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__); \
|
akerr_log_method("%s:%s:%d: Unable to pull an error context from the array!", __FILE__, (char *)__func__, __LINE__); \
|
||||||
exit(1); \
|
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 *
|
* 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__); \
|
snprintf((char *)__err_context->function, AKERR_MAX_ERROR_FUNCTION_LENGTH, __func__); \
|
||||||
__err_context->lineno = __LINE__; \
|
__err_context->lineno = __LINE__; \
|
||||||
snprintf((char *)__err_context->message, AKERR_MAX_ERROR_CONTEXT_STRING_LENGTH, __message, ## __VA_ARGS__); \
|
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) \
|
#define SUCCEED(__err_context) \
|
||||||
@@ -198,7 +220,7 @@ void akerr_init_errno(void);
|
|||||||
VALID(__err_context, __stmt); \
|
VALID(__err_context, __stmt); \
|
||||||
if ( __err_context != NULL ) { \
|
if ( __err_context != NULL ) { \
|
||||||
if ( __err_context->status != 0 ) { \
|
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; \
|
break; \
|
||||||
} \
|
} \
|
||||||
}
|
}
|
||||||
|
|||||||
51
tests/err_refcount_double_fail.c
Normal file
51
tests/err_refcount_double_fail.c
Normal file
@@ -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;
|
||||||
|
}
|
||||||
54
tests/err_stacktrace_bounds.c
Normal file
54
tests/err_stacktrace_bounds.c
Normal file
@@ -0,0 +1,54 @@
|
|||||||
|
#include "akerror.h"
|
||||||
|
#include "err_capture.h"
|
||||||
|
#include <string.h>
|
||||||
|
|
||||||
|
/*
|
||||||
|
* 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;
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user