Skip to content

Commit ab46288

Browse files
committed
fix potential mem corruption in utils_log_internal
1 parent c7fdae5 commit ab46288

2 files changed

Lines changed: 100 additions & 31 deletions

File tree

src/utils/utils_log.c

Lines changed: 67 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
*
3-
* Copyright (C) 2024-2025 Intel Corporation
3+
* Copyright (C) 2024-2026 Intel Corporation
44
*
55
* Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
66
* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
@@ -72,6 +72,40 @@ typedef struct {
7272
utils_log_config_t loggerConfig = {false, false, LOG_ERROR,
7373
LOG_ERROR, NULL, ""};
7474

75+
static void log_buffer_advance(char **pos, size_t *size, size_t written) {
76+
ASSERT(pos);
77+
ASSERT(size);
78+
79+
// do nothing if the size is 0 or can only hold the NULL terminator
80+
if (*size <= 1) {
81+
*size = 0;
82+
return;
83+
}
84+
85+
size_t advance = utils_min(written, *size - 1);
86+
*pos += advance;
87+
*size -= advance;
88+
}
89+
90+
static void log_buffer_append(char **pos, size_t *size, const char *src) {
91+
ASSERT(pos);
92+
ASSERT(size);
93+
ASSERT(src);
94+
95+
if (*size == 0) {
96+
return;
97+
}
98+
99+
size_t src_len = strlen(src);
100+
size_t copy_len = utils_min(src_len, *size - 1);
101+
memcpy(*pos, src, copy_len);
102+
103+
// copy_len excludes the trailing NULL, add it explicitly
104+
(*pos)[copy_len] = '\0';
105+
106+
log_buffer_advance(pos, size, src_len);
107+
}
108+
75109
static const char *level_to_str(utils_log_level_t l) {
76110
switch (l) {
77111
case LOG_DEBUG:
@@ -113,24 +147,29 @@ static void utils_log_internal(utils_log_level_t level, int perror,
113147

114148
char buffer[LOG_MAX];
115149
char *b_pos = buffer;
116-
int b_size = sizeof(buffer);
150+
size_t b_size = sizeof(buffer);
117151

118152
int tmp = 0;
119153
if (fileline == NULL) {
120154
tmp = snprintf(b_pos, b_size, "%s: ", func);
121155
} else {
122156
tmp = snprintf(b_pos, b_size, "%s %s: ", fileline, func);
123157
}
124-
ASSERT(tmp > 0);
125158

126-
b_pos += (int)tmp;
127-
b_size -= (int)tmp;
159+
if (tmp < 0) {
160+
return; // snprintf error
161+
}
162+
163+
size_t written = (size_t)tmp;
164+
log_buffer_advance(&b_pos, &b_size, written);
128165

129166
tmp = vsnprintf(b_pos, b_size, format, args);
130-
ASSERT(tmp > 0);
167+
if (tmp < 0) {
168+
return; // vsnprintf error
169+
}
131170

132-
b_pos += (int)tmp;
133-
b_size -= (int)tmp;
171+
written = (size_t)tmp;
172+
log_buffer_advance(&b_pos, &b_size, written);
134173

135174
const char *postfix = "";
136175

@@ -139,10 +178,12 @@ static void utils_log_internal(utils_log_level_t level, int perror,
139178
strncat(b_pos, ": ", b_size);
140179
b_pos += 2;
141180
b_size -= 2;
181+
const char *err = "";
142182
#if defined(_WIN32)
143-
char err[80]; // max size according to msdn
144-
if (strerror_s(err, sizeof(err), errno)) {
145-
*err = '\0';
183+
char err_buff[80]; // max size according to msdn
184+
err = err_buff;
185+
if (strerror_s(err_buff, sizeof(err_buff), errno)) {
186+
*err_buff = '\0';
146187
postfix = "[strerror_s failed]";
147188
}
148189
#else
@@ -151,33 +192,27 @@ static void utils_log_internal(utils_log_level_t level, int perror,
151192

152193
#if defined(__APPLE__) || \
153194
((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE)
154-
char err[1024];
155-
int ret = strerror_r(saveno, err, sizeof(err));
195+
char err_buff[1024];
196+
err = err_buff;
197+
int ret = strerror_r(saveno, err_buff, sizeof(err_buff));
156198
if (ret) {
157-
*err = '\0';
199+
*err_buff = '\0';
158200
postfix = "[strerror_r failed]";
159201
}
160202
if (errno == ERANGE) {
161203
postfix = "[truncated...]";
162204
}
163205
#else
164206
char err_buff[1024];
165-
const char *err = strerror_r(saveno, err_buff, sizeof(err_buff));
207+
err = strerror_r(saveno, err_buff, sizeof(err_buff));
166208
if (errno == ERANGE) {
167209
postfix = "[truncated...]";
168210
}
169211
#endif
170212

171213
errno = saveno;
172214
#endif
173-
strncpy(b_pos, err, b_size);
174-
size_t err_size = strlen(err);
175-
b_pos += err_size;
176-
b_size -= (int)err_size;
177-
if (b_size <= 0) {
178-
buffer[LOG_MAX - 1] =
179-
'\0'; //strncpy do not add \0 in case of overflow
180-
}
215+
log_buffer_append(&b_pos, &b_size, err);
181216
} else {
182217
postfix = "[truncated...]";
183218
}
@@ -190,7 +225,7 @@ static void utils_log_internal(utils_log_level_t level, int perror,
190225

191226
char header[LOG_HEADER];
192227
char *h_pos = header;
193-
int h_size = sizeof(header);
228+
size_t h_size = sizeof(header);
194229
memset(header, 0, sizeof(header));
195230

196231
if (loggerConfig.enableTimestamp) {
@@ -202,18 +237,19 @@ static void utils_log_internal(utils_log_level_t level, int perror,
202237
localtime_r(&now, &tm_info);
203238
#endif
204239

205-
ASSERT(h_size > 0);
206-
tmp = (int)strftime(h_pos, h_size, "%Y-%m-%dT%H:%M:%S ", &tm_info);
207-
h_pos += tmp;
208-
h_size -= tmp;
240+
written = strftime(h_pos, h_size, "%Y-%m-%dT%H:%M:%S ", &tm_info);
241+
log_buffer_advance(&h_pos, &h_size, written);
209242
}
210243

211244
if (loggerConfig.enablePid) {
212245
ASSERT(h_size > 0);
213246
tmp = snprintf(h_pos, h_size, "PID:%-6lu TID:%-6lu ",
214247
(unsigned long)pid, (unsigned long)tid);
215-
h_pos += tmp;
216-
h_size -= tmp;
248+
if (tmp < 0) {
249+
return; // snprintf error
250+
}
251+
written = (size_t)tmp;
252+
log_buffer_advance(&h_pos, &h_size, written);
217253
}
218254

219255
// We take twice header size here to ensure that
@@ -222,6 +258,7 @@ static void utils_log_internal(utils_log_level_t level, int perror,
222258
char logLine[LOG_MAX + LOG_HEADER * 2];
223259
snprintf(logLine, sizeof(logLine), "[%s%-5s UMF] %s%s\n", header,
224260
level_to_str(level), buffer, postfix);
261+
225262
FILE *out = loggerConfig.output ? loggerConfig.output : stderr;
226263
fputs(logLine, out);
227264

test/utils/utils_log.cpp

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (C) 2024-2025 Intel Corporation
1+
// Copyright (C) 2024-2026 Intel Corporation
22
// Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

@@ -368,6 +368,38 @@ TEST_F(test, long_log) {
368368
std::string(8190 - MOCK_FN_NAME.size(), 'x').c_str());
369369
}
370370

371+
TEST_F(test, long_func_name) {
372+
expect_fput_count = 1;
373+
expect_fflush_count = 1;
374+
loggerConfig =
375+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
376+
377+
std::string long_func(LOG_MAX, 'f');
378+
expected_message =
379+
"[DEBUG UMF] " + long_func.substr(0, LOG_MAX - 1) + "[truncated...]\n";
380+
381+
helper_test_log(LOG_DEBUG, long_func.c_str(), "%s", "tail");
382+
}
383+
384+
TEST_F(test, long_fileline_and_func_name) {
385+
expect_fput_count = 1;
386+
expect_fflush_count = 1;
387+
loggerConfig =
388+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
389+
390+
std::string long_fileline(LOG_MAX, 'l');
391+
std::string long_func(LOG_MAX, 'f');
392+
expected_message = "[DEBUG UMF] " + long_fileline.substr(0, LOG_MAX - 1) +
393+
"[truncated...]\n";
394+
395+
fput_count = 0;
396+
fflush_count = 0;
397+
utils_log(LOG_DEBUG, long_fileline.c_str(), long_func.c_str(), "%s",
398+
"tail");
399+
EXPECT_EQ(fput_count, expect_fput_count);
400+
EXPECT_EQ(fflush_count, expect_fflush_count);
401+
}
402+
371403
TEST_F(test, timestamp_log) {
372404
expect_fput_count = 1;
373405
expect_fflush_count = 1;

0 commit comments

Comments
 (0)