Protection access improvements 1 (#20093)
* Add support for nested protected access regions This change enhances the protected_access mechanism to support nested calls: - Replace single state with a stack of frames (up to 8 levels deep) - Modified signal handler to find and recover from the correct nesting level - Proper unwinding of frames when errors occur - Maintains original API and performance characteristics on the happy path * Move diagnostic functions to C file and update all PROTECTED_ACCESS_SETUP calls - Move protected_access_format_error and protected_access_get_last_fault from header to .c file - Fix remaining call in pagecache.c to use new API with resource name and operation - Use SIGNAL_CODE_2str_h directly for proper signal code printing - Clean up header file and improve implementation organization Add enhanced diagnostic information to protected access This change adds detailed diagnostic information to the protected access mechanism: - Extend API to require resource name and operation type for all callers - Capture signal codes using SIGNAL_CODE type with human-readable representations - Automatically log detailed error messages at the macro level - Format errors with contextual information about signal, memory region and offsets - Update journalfile.c to use the enhanced diagnostics
Costa Tsaousis committed
Apr 9, 2025 at 14:07 UTC
3145497b885582fdef9eb1adaf1f4ac40b393290
4 files changed
+148
-53
src/daemon/protected-access.c
+80
-29
@@ -8,6 +8,44 @@ __thread protected_access_t protected_access_state = {0};
8
// Declare the thread-local state variable, initialized to zero/inactive.
9
// *** RELIES ON ASYNC-SIGNAL-SAFE ACCESS TO THIS VARIABLE ***
10
11
+// Helper function to get diagnostic information from the last fault
12
+const protected_access_frame_t *protected_access_get_last_fault(void) {
13
+ if (protected_access_state.depth < 1)
14
+ return NULL;
15
+
16
+ protected_access_frame_t *frame = &protected_access_state.stack[protected_access_state.depth-1];
17
+ if (frame->is_active != 2) // Not a frame with a fault
18
+ return NULL;
19
+
20
+ return frame;
21
+}
22
+
23
+// Format a string with diagnostic information about the last fault
24
+void protected_access_format_error(char *buffer, size_t buffer_size) {
25
+ const protected_access_frame_t *frame = protected_access_get_last_fault();
26
+ if (!frame) {
27
+ snprintf(buffer, buffer_size, "No protected access fault information available");
28
+ return;
29
+ }
30
+
31
+ // Use the proper public API for signal code formatting
32
+ char signal_code_buf[128];
33
+ SIGNAL_CODE_2str_h(frame->signal_code, signal_code_buf, sizeof(signal_code_buf));
34
+
35
+ snprintf(buffer, buffer_size,
36
+ "Protected access fault in %s: %s %s failed with signal %s\n"
37
+ " Fault address: %p (offset +%lu within protected region %p-%p)",
38
+ frame->caller,
39
+ frame->operation,
40
+ frame->resource_name,
41
+ signal_code_buf,
42
+ frame->fault_address,
43
+ (unsigned long)((char*)frame->fault_address - (char*)frame->protected_start_addr),
44
+ frame->protected_start_addr,
45
+ (void*)((char*)frame->protected_start_addr + frame->protected_size)
46
+ );
47
+}
48
+
49
// --- Public API Function (called by signal handler) ---
50
void signal_protected_access_check(int sig, siginfo_t *si, void *context __maybe_unused) {
51
// --- ASYNC-SIGNAL-SAFETY WARNING ---
@@ -21,10 +59,9 @@ void signal_protected_access_check(int sig, siginfo_t *si, void *context __maybe
59
60
protected_access_t *state = &protected_access_state;
61
24
- // 1. Is protection currently active for *this thread*?
25
- // Check for state '1' specifically. Don't act if inactive ('0') or jump already happened ('2').
26
- if (state->is_active != 1)
27
- return; // Protection not active, handler should ignore.
62
+ // Make sure we have active frames
63
+ if (state->depth == 0)
64
+ return; // No protection active, handler should ignore.
65
66
// 2. Is it a signal we want to handle this way?
67
// Typically SIGBUS or SIGSEGV for memory access errors.
@@ -37,37 +74,51 @@ void signal_protected_access_check(int sig, siginfo_t *si, void *context __maybe
74
return; // This shouldn't happen if sigaction was set up correctly with SA_SIGINFO
75
76
void *fault_addr = si->si_addr;
40
- void *start_addr = state->protected_start_addr;
41
- // Perform boundary check carefully
42
- // Check if start_addr is valid before calculation
43
- if (start_addr == NULL) {
44
- // State inconsistency? Should not happen if is_active is 1.
45
- state->is_active = 0; // Attempt reset
46
- return;
47
- }
77
49
- // Calculate end address (exclusive)
50
- void *end_addr = (unsigned char *)start_addr + state->protected_size;
78
+ // Start from the most recent frame and work backwards
79
+ for (sig_atomic_t i = state->depth - 1; i >= 0; i--) {
80
+ protected_access_frame_t *frame = &state->stack[i];
81
+
82
+ // Skip inactive frames (shouldn't happen but check anyway)
83
+ if (frame->is_active != 1)
84
+ continue;
85
+
86
+ void *start_addr = frame->protected_start_addr;
87
+
88
+ // Check if start_addr is valid
89
+ if (start_addr == NULL)
90
+ continue;
91
+
92
+ // Calculate end address (exclusive)
93
+ void *end_addr = (unsigned char *)start_addr + frame->protected_size;
94
+
95
+ if (fault_addr >= start_addr && fault_addr < end_addr) {
96
+ // --- Conditions met! Perform recovery jump ---
97
52
- if (fault_addr >= start_addr && fault_addr < end_addr) {
53
- // --- Conditions met! Perform recovery jump ---
98
+ // Mark that recovery jump is occurring *before* jumping.
99
+ // Set frame to '2'. This prevents handler re-entry if another signal occurs
100
+ // immediately, and signals to start() that recovery happened.
101
+ frame->is_active = 2;
102
+
103
+ // Store diagnostic information about the fault
104
+ frame->fault_address = fault_addr;
105
+ frame->signal_code = signal_code(sig, si->si_code);
106
55
- // Mark that recovery jump is occurring *before* jumping.
56
- // Set state to '2'. This prevents handler re-entry if another signal occurs
57
- // immediately, and signals to start() that recovery happened.
58
- state->is_active = 2;
107
+ // Update the depth to unwind all nested frames up to this one
108
+ state->depth = i;
109
60
- // Jump back to the sigsetjmp point in signal_protected_access_start()
61
- // The '1' becomes the non-zero return value of sigsetjmp.
62
- siglongjmp(state->jump_buffer, 1);
110
+ // Jump back to the sigsetjmp point in PROTECTED_ACCESS_START
111
+ // The '1' becomes the non-zero return value of sigsetjmp.
112
+ siglongjmp(frame->jump_buffer, 1);
113
64
- // --- Execution should not reach here after siglongjmp ---
65
- // If it somehow did, something is fundamentally broken.
66
- fprintf(stderr, "FATAL: siglongjmp returned in signal handler!\n");
67
- abort();
68
- return; // Should be unreachable
114
+ // --- Execution should not reach here after siglongjmp ---
115
+ // If it somehow did, something is fundamentally broken.
116
+ fprintf(stderr, "FATAL: siglongjmp returned in signal handler!\n");
117
+ abort();
118
+ return; // Should be unreachable
119
+ }
120
}
121
71
- // Signal occurred while active, but fault address was outside the protected range.
122
+ // Signal occurred while active, but fault address was outside all protected ranges.
123
// Let the default handler deal with it.
124
}
src/daemon/protected-access.h
+63
-20
@@ -6,36 +6,63 @@
6
#include "libnetdata/libnetdata.h"
7
#include <setjmp.h>
8
9
+// Maximum nesting depth for protected access regions
10
+#define PROTECTED_ACCESS_MAX_NESTING 8
11
+
12
typedef struct {
10
- const char *caller;
13
+ const char *caller; // Function that initiated the protected region
14
sigjmp_buf jump_buffer; // Where to jump back to
15
void *protected_start_addr; // Start of the monitored memory range
16
size_t protected_size; // Size of the monitored memory range
17
// 0=inactive, 1=active (in protected block), 2=jump occurred
18
volatile sig_atomic_t is_active; // Must be sig_atomic_t for signal handler safety
19
+
20
+ // Enhanced diagnostic information
21
+ const char *resource_name; // Name/path of the resource being accessed (often a file path)
22
+ const char *operation; // Operation being performed (e.g. "read", "write", "mmap")
23
+ SIGNAL_CODE signal_code; // Signal and code combination (e.g. SIGSEGV/SEGV_MAPERR)
24
+ void *fault_address; // The exact address that caused the fault
25
+} protected_access_frame_t;
26
+
27
+typedef struct {
28
+ protected_access_frame_t stack[PROTECTED_ACCESS_MAX_NESTING];
29
+ volatile sig_atomic_t depth; // Current nesting depth (0 = no active protection)
30
} protected_access_t;
31
32
extern __thread protected_access_t protected_access_state;
33
20
-#define PROTECTED_ACCESS_START(start, size) ({ \
34
+#define PROTECTED_ACCESS_START(start, size, resource, op) ({ \
35
bool _rc = false; \
36
\
23
- if (protected_access_state.is_active == 1) \
24
- fatal("PROTECTED ACCESS: nested PROTECTED_ACCESS_START attempted from " \
25
- "function %s, while the active is from function %s!", \
26
- __FUNCTION__, protected_access_state.caller); \
37
+ if (protected_access_state.depth >= PROTECTED_ACCESS_MAX_NESTING) \
38
+ fatal("PROTECTED ACCESS: maximum nesting depth reached in function %s", \
39
+ __FUNCTION__); \
40
\
41
if (start && size) { \
29
- protected_access_state.protected_start_addr = start; \
30
- protected_access_state.protected_size = size; \
31
- protected_access_state.is_active = 1; \
32
- protected_access_state.caller = __FUNCTION__; \
33
- if (sigsetjmp(protected_access_state.jump_buffer, 1) == 0) { \
42
+ /* Get the current frame on the stack */ \
43
+ protected_access_frame_t *frame = \
44
+ &protected_access_state.stack[protected_access_state.depth]; \
45
+ \
46
+ /* Initialize the frame */ \
47
+ frame->protected_start_addr = start; \
48
+ frame->protected_size = size; \
49
+ frame->is_active = 1; \
50
+ frame->caller = __FUNCTION__; \
51
+ frame->resource_name = resource; \
52
+ frame->operation = op; \
53
+ frame->signal_code = 0; \
54
+ frame->fault_address = NULL; \
55
+ \
56
+ /* Increase the stack depth before setting up the jump */ \
57
+ protected_access_state.depth++; \
58
+ \
59
+ if (sigsetjmp(frame->jump_buffer, 1) == 0) { \
60
/* Initial call successful, sigsetjmp returns 0. */ \
61
_rc = true; \
62
} else { \
63
/* Returned here via siglongjmp from the signal handler. */ \
38
- /* The handler should have set state->is_active = 2. */ \
64
+ /* The handler should have set frame->is_active = 2 */ \
65
+ /* and populated diagnostic information. */ \
66
/* Return false to indicate recovery path should be taken. */ \
67
_rc = false; \
68
} \
@@ -44,21 +71,37 @@ extern __thread protected_access_t protected_access_state;
71
})
72
73
static inline void protected_access_end(volatile int *ptr __maybe_unused) {
47
- protected_access_state.is_active = 0;
48
- protected_access_state.protected_start_addr = NULL;
49
- protected_access_state.protected_size = 0;
50
- /* No need to clear jump_buffer explicitly */
74
+ if (protected_access_state.depth > 0) {
75
+ /* Decrease the stack depth */
76
+ protected_access_state.depth--;
77
+
78
+ /* Clear the frame at the current depth */
79
+ protected_access_frame_t *frame = &protected_access_state.stack[protected_access_state.depth];
80
+ frame->is_active = 0;
81
+ frame->protected_start_addr = NULL;
82
+ frame->protected_size = 0;
83
+ /* No need to clear jump_buffer explicitly */
84
+ }
85
}
86
53
-#define PROTECTED_ACCESS_AUTO_CLEANUP() \
54
- volatile int _pa_dummy_cleanup_var __attribute__((cleanup(protected_access_end), unused)) = 0; \
87
+#define PROTECTED_ACCESS_AUTO_CLEANUP() \
88
+ volatile int _pa_dummy_cleanup_var __attribute__((cleanup(protected_access_end), unused)) = 0;
89
90
#define PROTECTED_ACCESS_END() protected_access_end(NULL);
91
58
-#define PROTECTED_ACCESS_SETUP(start, size) \
92
+#define PROTECTED_ACCESS_SETUP(start, size, resource, op) \
93
PROTECTED_ACCESS_AUTO_CLEANUP(); \
60
- bool no_signal_received = PROTECTED_ACCESS_START(start, size); \
94
+ bool no_signal_received = PROTECTED_ACCESS_START(start, size, resource, op); \
95
+ if (!no_signal_received) { \
96
+ char __pa_error_buf[1024]; \
97
+ protected_access_format_error(__pa_error_buf, sizeof(__pa_error_buf)); \
98
+ nd_log(NDLS_DAEMON, NDLP_ERR, "%s", __pa_error_buf); \
99
+ }
100
101
void signal_protected_access_check(int sig, siginfo_t *si, void *context);
102
103
+// Function declarations for diagnostic functions
104
+const protected_access_frame_t *protected_access_get_last_fault(void);
105
+void protected_access_format_error(char *buffer, size_t buffer_size);
106
+
107
#endif // PROTECTED_ACCESS_H
src/database/engine/journalfile.c
+2
-3
@@ -1105,12 +1105,11 @@ int journalfile_v2_load(struct rrdengine_instance *ctx, struct rrdengine_journal
1105
usec_t validation_start_ut = now_monotonic_usec();
1106
1107
int rc = 0;
1108
- PROTECTED_ACCESS_SETUP(data_start, journal_v2_file_size);
1108
+ PROTECTED_ACCESS_SETUP(data_start, journal_v2_file_size, path_v2, "validate");
1109
if(no_signal_received) {
1110
rc = journalfile_v2_validate(data_start, journal_v2_file_size, journal_v1_file_size);
1111
}
1112
else {
1113
- nd_log(NDLS_DAEMON, NDLP_ERR, "DBENGINE: failed to access journal file '%s' (SIGBUS)", path_v2);
1113
rc = 2;
1114
}
1115
@@ -1364,7 +1363,7 @@ bool journalfile_migrate_to_v2_callback(Word_t section, unsigned datafile_fileno
1363
1364
struct journal_metric_list_to_sort *uuid_list = NULL;
1365
1367
- PROTECTED_ACCESS_SETUP(data_start, total_file_size);
1366
+ PROTECTED_ACCESS_SETUP(data_start, total_file_size, path, "migrate");
1367
if(no_signal_received) {
1368
fatal_assert(extent_offset <= total_file_size);
1369
memset(data_start, 0, extent_offset);
src/database/engine/pagecache.c
+3
-1
@@ -513,7 +513,9 @@ static NOT_INLINE_HOT size_t get_page_list_from_journal_v2(struct rrdengine_inst
513
if (unlikely(!j2_header))
514
continue;
515
516
- PROTECTED_ACCESS_SETUP(datafile->journalfile->mmap.data, datafile->journalfile->mmap.size);
516
+ char file_path[RRDENG_PATH_MAX];
517
+ journalfile_v2_generate_path(datafile, file_path, sizeof(file_path));
518
+ PROTECTED_ACCESS_SETUP(datafile->journalfile->mmap.data, datafile->journalfile->mmap.size, file_path, "read");
519
if(no_signal_received) {
520
time_t journal_start_time_s = (time_t)(j2_header->start_time_ut / USEC_PER_SEC);
521
size_t journal_v2_file_size = datafile->journalfile->mmap.size;