Skip to content

Commit 15922c5

Browse files
committed
fix potential mem corruption in utils_log_internal
1 parent 10c33e0 commit 15922c5

2 files changed

Lines changed: 97 additions & 25 deletions

File tree

src/utils/utils_log.c

Lines changed: 64 additions & 24 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,54 @@ 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, int *size, int written) {
76+
ASSERT(pos);
77+
ASSERT(size);
78+
79+
if (*size <= 0) {
80+
return;
81+
}
82+
83+
if (written < 0) {
84+
*size = 0;
85+
return;
86+
}
87+
88+
// advance the buffer position and decrease the remaining size
89+
// take into the actual number of characters written
90+
if (written >= *size) {
91+
*pos += *size - 1;
92+
*size = 0;
93+
return;
94+
}
95+
96+
*pos += written;
97+
*size -= written;
98+
}
99+
100+
static void log_buffer_append(char **pos, int *size, const char *src) {
101+
ASSERT(pos);
102+
ASSERT(size);
103+
ASSERT(src);
104+
105+
if (*size <= 0) {
106+
return;
107+
}
108+
109+
size_t src_len = strlen(src);
110+
size_t copy_len = src_len;
111+
112+
if ((int)copy_len >= *size) {
113+
copy_len = (size_t)(*size - 1);
114+
}
115+
memcpy(*pos, src, copy_len);
116+
117+
// ensure null-termination
118+
(*pos)[copy_len] = '\0';
119+
120+
log_buffer_advance(pos, size, (int)src_len);
121+
}
122+
75123
static const char *level_to_str(utils_log_level_t l) {
76124
switch (l) {
77125
case LOG_DEBUG:
@@ -123,14 +171,12 @@ static void utils_log_internal(utils_log_level_t level, int perror,
123171
}
124172
ASSERT(tmp > 0);
125173

126-
b_pos += (int)tmp;
127-
b_size -= (int)tmp;
174+
log_buffer_advance(&b_pos, &b_size, tmp);
128175

129176
tmp = vsnprintf(b_pos, b_size, format, args);
130177
ASSERT(tmp > 0);
131178

132-
b_pos += (int)tmp;
133-
b_size -= (int)tmp;
179+
log_buffer_advance(&b_pos, &b_size, tmp);
134180

135181
const char *postfix = "";
136182

@@ -139,10 +185,12 @@ static void utils_log_internal(utils_log_level_t level, int perror,
139185
strncat(b_pos, ": ", b_size);
140186
b_pos += 2;
141187
b_size -= 2;
188+
const char *err = "";
142189
#if defined(_WIN32)
143-
char err[80]; // max size according to msdn
144-
if (strerror_s(err, sizeof(err), errno)) {
145-
*err = '\0';
190+
char err_buff[80]; // max size according to msdn
191+
err = err_buff;
192+
if (strerror_s(err_buff, sizeof(err_buff), errno)) {
193+
*err_buff = '\0';
146194
postfix = "[strerror_s failed]";
147195
}
148196
#else
@@ -151,33 +199,27 @@ static void utils_log_internal(utils_log_level_t level, int perror,
151199

152200
#if defined(__APPLE__) || \
153201
((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE)
154-
char err[1024];
155-
int ret = strerror_r(saveno, err, sizeof(err));
202+
char err_buff[1024];
203+
err = err_buff;
204+
int ret = strerror_r(saveno, err_buff, sizeof(err_buff));
156205
if (ret) {
157-
*err = '\0';
206+
*err_buff = '\0';
158207
postfix = "[strerror_r failed]";
159208
}
160209
if (errno == ERANGE) {
161210
postfix = "[truncated...]";
162211
}
163212
#else
164213
char err_buff[1024];
165-
const char *err = strerror_r(saveno, err_buff, sizeof(err_buff));
214+
err = strerror_r(saveno, err_buff, sizeof(err_buff));
166215
if (errno == ERANGE) {
167216
postfix = "[truncated...]";
168217
}
169218
#endif
170219

171220
errno = saveno;
172221
#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-
}
222+
log_buffer_append(&b_pos, &b_size, err);
181223
} else {
182224
postfix = "[truncated...]";
183225
}
@@ -204,16 +246,14 @@ static void utils_log_internal(utils_log_level_t level, int perror,
204246

205247
ASSERT(h_size > 0);
206248
tmp = (int)strftime(h_pos, h_size, "%Y-%m-%dT%H:%M:%S ", &tm_info);
207-
h_pos += tmp;
208-
h_size -= tmp;
249+
log_buffer_advance(&h_pos, &h_size, tmp);
209250
}
210251

211252
if (loggerConfig.enablePid) {
212253
ASSERT(h_size > 0);
213254
tmp = snprintf(h_pos, h_size, "PID:%-6lu TID:%-6lu ",
214255
(unsigned long)pid, (unsigned long)tid);
215-
h_pos += tmp;
216-
h_size -= tmp;
256+
log_buffer_advance(&h_pos, &h_size, tmp);
217257
}
218258

219259
// We take twice header size here to ensure that

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)