Skip to content

Commit 1fb4bf9

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 Conflicts: agent/src/ebpf/user/log.c agent/src/ebpf/user/profile/stringifier.c
1 parent bb7da1c commit 1fb4bf9

11 files changed

Lines changed: 359 additions & 122 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: 32 additions & 19 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
FILE *log_stream;
2728
bool log_to_stdout;
@@ -79,7 +80,7 @@ __attribute__((weak)) void rust_info_wrapper(char *msg)
7980
printf("%s\n", msg);
8081
}
8182

82-
static char *dispatch_message(char *msg, uint16_t len)
83+
static char *dispatch_message(char *msg, size_t len)
8384
{
8485
if (!msg || len < 1)
8586
return msg;
@@ -94,28 +95,38 @@ void _ebpf_error(int how_to_die, char *function_name, char *file_path,
9495
uint32_t line_number, char *fmt, ...)
9596
{
9697
char msg[MSG_SZ] = {};
97-
uint16_t len = 0;
98-
uint16_t max = MSG_SZ;
98+
size_t len = 0;
99+
int64_t remaining;
99100
va_list va;
100101

101102
if (function_name) {
103+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
102104
if (how_to_die & ERROR_WARNING) {
103-
len += snprintf(msg + len, max - len, "[eBPF] WARN func %s()", function_name);
105+
len += safe_snprintf(msg + len, remaining,
106+
"[eBPF] WARN func %s()",
107+
function_name);
104108
} else {
105-
len += snprintf(msg + len, max - len, "[eBPF] ERROR func %s()", function_name);
109+
len += safe_snprintf(msg + len, remaining,
110+
"[eBPF] ERROR func %s()",
111+
function_name);
112+
}
113+
if (line_number > 0) {
114+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
115+
len += safe_snprintf(msg + len, remaining, " [%s:%u] ",
116+
file_path, line_number);
106117
}
107-
if (line_number > 0)
108-
len +=
109-
snprintf(msg + len, max - len, " [%s:%u] ",
110-
file_path, line_number);
111118
}
112119
#ifdef HAVE_ERRNO
113-
if (how_to_die & ERROR_ERRNO_VALID)
114-
len += snprintf(msg + len, max - len,
115-
": %s (errno %d)", strerror(errno), errno);
120+
if (how_to_die & ERROR_ERRNO_VALID) {
121+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
122+
len += safe_snprintf(msg + len, remaining,
123+
": %s (errno %d)", strerror(errno),
124+
errno);
125+
}
116126
#endif
117127
va_start(va, fmt);
118-
len += vsnprintf(msg + len, max - len, fmt, va);
128+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
129+
len += safe_vsnprintf(msg + len, remaining, fmt, va);
119130
va_end(va);
120131

121132
dispatch_message(msg, len);
@@ -129,17 +140,19 @@ void _ebpf_error(int how_to_die, char *function_name, char *file_path,
129140
void _ebpf_info(char *fmt, ...)
130141
{
131142
char msg[MSG_SZ] = {};
132-
uint16_t len = 0;
133-
uint16_t max = MSG_SZ;
143+
size_t len = 0;
144+
int64_t remaining;
134145
va_list va;
135146

136-
len += snprintf(msg + len, max - len, "[eBPF] INFO ");
147+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
148+
len += safe_snprintf(msg + len, remaining, "[eBPF] INFO ");
137149

138150
va_start(va, fmt);
139-
len += vsnprintf(msg + len, max - len, fmt, va);
151+
remaining = (int64_t)sizeof(msg) - (int64_t)len;
152+
len += safe_vsnprintf(msg + len, remaining, fmt, va);
140153
va_end(va);
141-
if (msg[len - 1] != '\n') {
142-
if (len < max)
154+
if (len > 0 && msg[len - 1] != '\n') {
155+
if (len < sizeof(msg))
143156
msg[len++] = '\n';
144157
else
145158
msg[len - 1] = '\n';

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
@@ -1106,27 +1106,31 @@ static void aggregate_stack_traces(struct profiler_context *ctx,
11061106

11071107
char *msg_str = (char *)&msg->data[0];
11081108
int offset = 0;
1109+
int64_t remaining;
11091110
if (matched) {
1110-
offset +=
1111-
snprintf(msg_str + offset, str_len - offset,
1112-
"%s%s;", pre_tag, v->comm);
1111+
remaining = (int64_t)str_len - offset;
1112+
offset += safe_snprintf(msg_str + offset,
1113+
remaining, "%s%s;",
1114+
pre_tag, v->comm);
11131115
}
1114-
offset +=
1115-
snprintf(msg_str + offset, str_len - offset, "%s",
1116-
trace_str);
1116+
remaining = (int64_t)str_len - offset;
1117+
offset += safe_snprintf(msg_str + offset, remaining, "%s",
1118+
trace_str);
11171119
if (ctx->type == PROFILER_TYPE_MEMORY && v->memory.class_id != 0) {
11181120
if (class_name) {
1119-
offset +=
1120-
snprintf(msg_str + offset,
1121-
str_len - offset, ";%s",
1122-
class_name);
1121+
remaining = (int64_t)str_len - offset;
1122+
offset += safe_snprintf(msg_str + offset,
1123+
remaining,
1124+
";%s",
1125+
class_name);
11231126
clib_mem_free(class_name);
11241127
class_name = NULL;
11251128
} else {
1126-
offset +=
1127-
snprintf(msg_str + offset,
1128-
str_len - offset, ";%s",
1129-
UNKNOWN_JAVA_SYMBOL_STR);
1129+
remaining = (int64_t)str_len - offset;
1130+
offset += safe_snprintf(msg_str + offset,
1131+
remaining,
1132+
";%s",
1133+
UNKNOWN_JAVA_SYMBOL_STR);
11301134
}
11311135
}
11321136

0 commit comments

Comments
 (0)