Skip to content

Commit 3db6360

Browse files
committed
fix: harden ebpf snprintf handling (#11755)
* fix: harden ebpf snprintf handling Fix incorrect snprintf/vsnprintf return-value semantics in eBPF user-space code. Add safe_snprintf/safe_vsnprintf helpers that consistently return the actual written length, and avoid treating snprintf's intended output length as the actual written length when it is later used for: - offset accumulation - remaining buffer size calculation - send length propagation Also fix the affected paths in: - log message assembly - Java symbol collection and log sending - stringifier string assembly - socket datadump/http2/grpc/io-event formatting - agent so path construction under target namespaces Also fix one case where a dynamic string was used directly as a format string. * test: add safe snprintf regression coverage Add a focused eBPF test that compares snprintf and safe_snprintf for exact-fit, truncation, and offset-accumulation cases. Also fix the socket datadump branch structure around safe_snprintf usage so the remaining-length calculation and write stay in the same conditional branch. * fix: finish ebpf snprintf follow-up Switch the remaining stack-trace string assembly in profile_common.c from offset-based snprintf accumulation to safe_snprintf, and avoid using map_name directly as a format string in tracer.c. Also include the current ebpf test Makefile updates in this follow-up. Conflicts: agent/src/ebpf/test/Makefile
1 parent 44204e1 commit 3db6360

11 files changed

Lines changed: 375 additions & 128 deletions

File tree

agent/src/ebpf/test/Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ ARCH ?= $(shell uname -m)
2424
CC ?= gcc
2525
CFLAGS ?= -std=gnu99 --static -g -O2 -ffunction-sections -fdata-sections -fPIC -fno-omit-frame-pointer -Wall -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers
2626

27-
EXECS := test_symbol test_offset test_insns_cnt test_bihash test_vec test_fetch_container_id test_parse_range test_set_ports_bitmap test_pid_check test_match_pids
27+
EXECS := test_symbol test_offset test_insns_cnt test_bihash test_vec test_fetch_container_id test_parse_range test_set_ports_bitmap test_match_pids test_safe_snprintf
2828
ifeq ($(ARCH), x86_64)
2929
#-lbcc -lstdc++
3030
LDLIBS += ../libtrace.a ./libtrace_utils.a -ljattach -lbcc_bpf -lGoReSym -lbddisasm -ldwarf -lelf -lz -lpthread -lbcc -lstdc++ -ldl -lm
@@ -38,7 +38,7 @@ all: $(EXECS) libtrace_utils.a
3838
$(call msg,TEST,$@)
3939
echo "$(Q)$(CC) $(CFLAGS) -o $@ $^ $(LDLIBS)"
4040
$(Q)$(CC) $(CFLAGS) -o $@ $^ $(LDLIBS)
41-
# $(Q)./$@
41+
$(Q)./$@
4242

4343
libtrace_utils.a:
4444
$(Q)cargo rustc -p trace-utils --crate-type=staticlib
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/*
2+
* Copyright (c) 2026 Yunshan Networks
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include <stdio.h>
18+
#include <string.h>
19+
20+
#include "../user/utils.h"
21+
22+
static int check_exact_fill(void)
23+
{
24+
char raw[6] = { 0 };
25+
char safe[6] = { 0 };
26+
int raw_len = snprintf(raw, sizeof(raw), "%s", "hello");
27+
size_t safe_len = safe_snprintf(safe, sizeof(safe), "%s", "hello");
28+
29+
if (raw_len != 5 || safe_len != 5) {
30+
printf("exact fill length mismatch: raw=%d safe=%zu\n",
31+
raw_len, safe_len);
32+
return -1;
33+
}
34+
35+
if (strcmp(raw, "hello") != 0 || strcmp(safe, "hello") != 0) {
36+
printf("exact fill buffer mismatch: raw=\"%s\" safe=\"%s\"\n",
37+
raw, safe);
38+
return -1;
39+
}
40+
41+
return 0;
42+
}
43+
44+
static int check_truncation(void)
45+
{
46+
char raw[4] = { 0 };
47+
char safe[4] = { 0 };
48+
int raw_len = snprintf(raw, sizeof(raw), "%s", "hello");
49+
size_t safe_len = safe_snprintf(safe, sizeof(safe), "%s", "hello");
50+
51+
if (raw_len != 5) {
52+
printf("unexpected raw truncation length: %d\n", raw_len);
53+
return -1;
54+
}
55+
56+
if (safe_len != sizeof(safe) - 1) {
57+
printf("unexpected safe truncation length: %zu\n", safe_len);
58+
return -1;
59+
}
60+
61+
if (strcmp(raw, "hel") != 0 || strcmp(safe, "hel") != 0) {
62+
printf("truncation buffer mismatch: raw=\"%s\" safe=\"%s\"\n",
63+
raw, safe);
64+
return -1;
65+
}
66+
67+
return 0;
68+
}
69+
70+
static int check_offset_accumulation(void)
71+
{
72+
char raw[4] = { 0 };
73+
char safe[4] = { 0 };
74+
int raw_len = 0;
75+
size_t safe_len = 0;
76+
77+
raw_len += snprintf(raw + raw_len, sizeof(raw) - raw_len, "%s", "hello");
78+
safe_len += safe_snprintf(safe + safe_len,
79+
(int64_t)sizeof(safe) - (int64_t)safe_len,
80+
"%s", "hello");
81+
82+
if (raw_len < sizeof(raw)) {
83+
printf("raw offset unexpectedly remained in bounds: %d\n", raw_len);
84+
return -1;
85+
}
86+
87+
if (safe_len >= sizeof(safe)) {
88+
printf("safe offset escaped buffer: %zu\n", safe_len);
89+
return -1;
90+
}
91+
92+
if (safe_len != sizeof(safe) - 1 || strcmp(safe, "hel") != 0) {
93+
printf("safe offset accumulation mismatch: len=%zu buf=\"%s\"\n",
94+
safe_len, safe);
95+
return -1;
96+
}
97+
98+
/*
99+
* The next append would be unsafe with raw_len because it already exceeds
100+
* the valid offset range after truncation. safe_len remains a valid offset.
101+
*/
102+
safe_len += safe_snprintf(safe + safe_len,
103+
(int64_t)sizeof(safe) - (int64_t)safe_len,
104+
"%s", "!");
105+
if (safe_len != sizeof(safe) - 1 || strcmp(safe, "hel") != 0) {
106+
printf("safe follow-up append changed truncated buffer: len=%zu buf=\"%s\"\n",
107+
safe_len, safe);
108+
return -1;
109+
}
110+
111+
return 0;
112+
}
113+
114+
int main(void)
115+
{
116+
if (check_exact_fill() != 0)
117+
return -1;
118+
119+
if (check_truncation() != 0)
120+
return -1;
121+
122+
if (check_offset_accumulation() != 0)
123+
return -1;
124+
125+
printf("[OK]\n");
126+
return 0;
127+
}

agent/src/ebpf/user/log.c

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <stdlib.h>
2323
#include <time.h>
2424
#include "log.h"
25+
#include "utils.h"
2526

2627
#include "trace_utils.h"
2728

@@ -81,7 +82,7 @@ __attribute__((weak)) void rust_info_wrapper(char *msg)
8182
printf("%s\n", msg);
8283
}
8384

84-
static char *dispatch_message(char *msg, uint16_t len)
85+
static char *dispatch_message(char *msg, size_t len)
8586
{
8687
if (!msg || len < 1)
8788
return msg;
@@ -96,28 +97,38 @@ void _ebpf_error(int how_to_die, char *function_name, char *file_path,
9697
uint32_t line_number, char *fmt, ...)
9798
{
9899
char msg[MSG_SZ] = {};
99-
uint16_t len = 0;
100-
uint16_t max = MSG_SZ;
100+
size_t len = 0;
101+
int64_t remaining;
101102
va_list va;
102103

103104
if (function_name) {
105+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
104106
if (how_to_die & ERROR_WARNING) {
105-
len += snprintf(msg + len, max - len, "[eBPF] WARN func %s()", function_name);
107+
len += safe_snprintf(msg + len, remaining,
108+
"[eBPF] WARN func %s()",
109+
function_name);
106110
} else {
107-
len += snprintf(msg + len, max - len, "[eBPF] ERROR func %s()", function_name);
111+
len += safe_snprintf(msg + len, remaining,
112+
"[eBPF] ERROR func %s()",
113+
function_name);
114+
}
115+
if (line_number > 0) {
116+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
117+
len += safe_snprintf(msg + len, remaining, " [%s:%u] ",
118+
file_path, line_number);
108119
}
109-
if (line_number > 0)
110-
len +=
111-
snprintf(msg + len, max - len, " [%s:%u] ",
112-
file_path, line_number);
113120
}
114121
#ifdef HAVE_ERRNO
115-
if (how_to_die & ERROR_ERRNO_VALID)
116-
len += snprintf(msg + len, max - len,
117-
": %s (errno %d)", strerror(errno), errno);
122+
if (how_to_die & ERROR_ERRNO_VALID) {
123+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
124+
len += safe_snprintf(msg + len, remaining,
125+
": %s (errno %d)", strerror(errno),
126+
errno);
127+
}
118128
#endif
119129
va_start(va, fmt);
120-
len += vsnprintf(msg + len, max - len, fmt, va);
130+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
131+
len += safe_vsnprintf(msg + len, remaining, fmt, va);
121132
va_end(va);
122133

123134
dispatch_message(msg, len);
@@ -131,17 +142,19 @@ void _ebpf_error(int how_to_die, char *function_name, char *file_path,
131142
void _ebpf_info(char *fmt, ...)
132143
{
133144
char msg[MSG_SZ] = {};
134-
uint16_t len = 0;
135-
uint16_t max = MSG_SZ;
145+
size_t len = 0;
146+
int64_t remaining;
136147
va_list va;
137148

138-
len += snprintf(msg + len, max - len, "[eBPF] INFO ");
149+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
150+
len += safe_snprintf(msg + len, remaining, "[eBPF] INFO ");
139151

140152
va_start(va, fmt);
141-
len += vsnprintf(msg + len, max - len, fmt, va);
153+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
154+
len += safe_vsnprintf(msg + len, remaining, fmt, va);
142155
va_end(va);
143-
if (msg[len - 1] != '\n') {
144-
if (len < max)
156+
if (len > 0 && msg[len - 1] != '\n') {
157+
if (len < sizeof(msg))
145158
msg[len++] = '\n';
146159
else
147160
msg[len - 1] = '\n';
@@ -165,16 +178,23 @@ void _ebpf_log(int how_to_die, char *function_name, char *file_path,
165178
{
166179
char msg[MSG_SZ] = {};
167180
va_list va;
181+
int ret;
182+
size_t len;
168183

169184
va_start(va, fmt);
170-
uint16_t len = vsnprintf(msg, MSG_SZ, fmt, va);
185+
ret = vsnprintf(msg, MSG_SZ, fmt, va);
171186
va_end(va);
172187

188+
if (ret < 0) {
189+
len = 0;
190+
} else {
191+
len = ret;
192+
}
173193
if (len >= MSG_SZ) {
174194
len = MSG_SZ - 1;
175195
}
176196
// remove trailing newline if any
177-
if (msg[len - 1] == '\n') {
197+
if (len > 0 && msg[len - 1] == '\n') {
178198
msg[len - 1] = '\0';
179199
}
180200

agent/src/ebpf/user/profile/java/jvm_symbol_collect.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,12 @@ static int copy_agent_libs_into_target_ns(pid_t target_pid, int target_uid,
12211221
char copy_target_path[MAX_PATH_LENGTH];
12221222
int len = snprintf(copy_target_path, sizeof(copy_target_path),
12231223
TARGET_NS_STORAGE_PATH, target_pid);
1224+
if (len < 0 || len >= sizeof(copy_target_path)) {
1225+
ebpf_warning(JAVA_LOG_TAG
1226+
"Fun %s target ns path is too long for pid %d\n",
1227+
__func__, target_pid);
1228+
return ETR_INVAL;
1229+
}
12241230
if (access(copy_target_path, F_OK)) {
12251231
/*
12261232
* The purpose of umask(0); is to set the current process's file
@@ -1240,8 +1246,9 @@ static int copy_agent_libs_into_target_ns(pid_t target_pid, int target_uid,
12401246
}
12411247
}
12421248

1243-
snprintf(copy_target_path + len, sizeof(copy_target_path) - len,
1244-
"/%s", AGENT_LIB_NAME);
1249+
safe_snprintf(copy_target_path + len,
1250+
(int64_t)sizeof(copy_target_path) - len, "/%s",
1251+
AGENT_LIB_NAME);
12451252
if ((ret =
12461253
agent_so_lib_copy(AGENT_LIB_SRC_PATH,
12471254
copy_target_path, target_uid,
@@ -1251,8 +1258,9 @@ static int copy_agent_libs_into_target_ns(pid_t target_pid, int target_uid,
12511258
return ret;
12521259
}
12531260

1254-
snprintf(copy_target_path + len, sizeof(copy_target_path) - len,
1255-
"/%s", AGENT_MUSL_LIB_NAME);
1261+
safe_snprintf(copy_target_path + len,
1262+
(int64_t)sizeof(copy_target_path) - len, "/%s",
1263+
AGENT_MUSL_LIB_NAME);
12561264

12571265
if ((ret =
12581266
agent_so_lib_copy(AGENT_MUSL_LIB_SRC_PATH,

agent/src/ebpf/user/profile/java/symbol_collect_agent.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include <jvmticmlr.h>
4040

4141
#include "../../config.h"
42+
#include "../../utils.h"
4243
#include "config.h"
4344

4445
#define LOG_BUF_SZ 512
@@ -80,7 +81,7 @@ jint close_files(void);
8081
do { \
8182
if (perf_map_log_socket_fd > 0) { \
8283
char str_buf[LOG_BUF_SZ]; \
83-
int n = snprintf(str_buf, sizeof(str_buf), format, ##__VA_ARGS__); \
84+
size_t n = safe_snprintf(str_buf, sizeof(str_buf), format, ##__VA_ARGS__); \
8485
pthread_mutex_lock(&g_df_lock); \
8586
send_msg(perf_map_log_socket_fd, str_buf, n); \
8687
pthread_mutex_unlock(&g_df_lock); \

agent/src/ebpf/user/profile/profile_common.c

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,27 +1110,31 @@ static void aggregate_stack_traces(struct profiler_context *ctx,
11101110

11111111
char *msg_str = (char *)&msg->data[0];
11121112
int offset = 0;
1113+
int64_t remaining;
11131114
if (matched) {
1114-
offset +=
1115-
snprintf(msg_str + offset, str_len - offset,
1116-
"%s%s;", pre_tag, v->comm);
1115+
remaining = (int64_t)str_len - offset;
1116+
offset += safe_snprintf(msg_str + offset,
1117+
remaining, "%s%s;",
1118+
pre_tag, v->comm);
11171119
}
1118-
offset +=
1119-
snprintf(msg_str + offset, str_len - offset, "%s",
1120-
trace_str);
1120+
remaining = (int64_t)str_len - offset;
1121+
offset += safe_snprintf(msg_str + offset, remaining, "%s",
1122+
trace_str);
11211123
if (ctx->type == PROFILER_TYPE_MEMORY && v->memory.class_id != 0) {
11221124
if (class_name) {
1123-
offset +=
1124-
snprintf(msg_str + offset,
1125-
str_len - offset, ";%s",
1126-
class_name);
1125+
remaining = (int64_t)str_len - offset;
1126+
offset += safe_snprintf(msg_str + offset,
1127+
remaining,
1128+
";%s",
1129+
class_name);
11271130
clib_mem_free(class_name);
11281131
class_name = NULL;
11291132
} else {
1130-
offset +=
1131-
snprintf(msg_str + offset,
1132-
str_len - offset, ";%s",
1133-
UNKNOWN_JAVA_SYMBOL_STR);
1133+
remaining = (int64_t)str_len - offset;
1134+
offset += safe_snprintf(msg_str + offset,
1135+
remaining,
1136+
";%s",
1137+
UNKNOWN_JAVA_SYMBOL_STR);
11341138
}
11351139
}
11361140

0 commit comments

Comments
 (0)