Skip to content

Commit f956a8a

Browse files
vgrebenschikovvgrebenschikov
authored
Fix #799 - memory leak in *statsd (#800)
1 parent 1e5e894 commit f956a8a

File tree

4 files changed

+248
-455
lines changed

4 files changed

+248
-455
lines changed

‎tools/src/racct-bhyve-statsd.c

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,13 @@ sum_data_bhyve()
225225
gettimeofday(&now_time, NULL);
226226
cur_time = (time_t)now_time.tv_sec;
227227

228+
// First, free existing sum_item_list
229+
for (sumch = sum_item_list; sumch; sumch = next_sumch) {
230+
next_sumch = sumch->next;
231+
free(sumch);
232+
}
233+
sum_item_list = NULL;
234+
228235
for (ch = item_list; ch; ch = ch->next) {
229236
if (ch->modified == 0) {
230237
continue;
@@ -253,6 +260,10 @@ sum_data_bhyve()
253260
}
254261
} else {
255262
CREATE(newd, struct sum_item_data, 1);
263+
if (!newd) {
264+
tolog(log_level, "Failed to allocate memory for newd\n");
265+
continue;
266+
}
256267
newd->modified = ch->modified;
257268
newd->pcpu = ch->pcpu;
258269
newd->memoryuse = ch->memoryuse;
@@ -290,7 +301,7 @@ sum_data_bhyve()
290301

291302
if (OUTPUT_BEANSTALKD & output_flags) {
292303
memset(json_buf, 0, sizeof(json_buf));
293-
sprintf(json_buf,
304+
snprintf(json_buf, sizeof(json_buf),
294305
"{\"name\": \"%s\",\"time\": %d,\"pcpu\": %d,\"pmem\": %d,\"readbps\": %d,\"writebps\": %d,\"readiops\": %d,\"writeiops\": %d }",
295306
sumch->name, cur_time, sumch->pcpu / round_total,
296307
sumch->pmem / round_total,
@@ -300,8 +311,13 @@ sum_data_bhyve()
300311
sumch->writeiops / round_total);
301312

302313
if (strlen(json_str) > 2) {
303-
strcat(json_str, ",");
304-
strcat(json_str, json_buf);
314+
if (strlen(json_str) + strlen(json_buf) + 2 < sizeof(json_str)) {
315+
strcat(json_str, ",");
316+
strcat(json_str, json_buf);
317+
} else {
318+
tolog(log_level, "Buffer overflow in json_str\n");
319+
break;
320+
}
305321
} else {
306322
strcpy(json_str,
307323
"{ \"tube\":\"racct-bhyve\", \"data\":[");
@@ -311,7 +327,8 @@ sum_data_bhyve()
311327

312328
#ifdef WITH_INFLUX
313329
if (OUTPUT_INFLUX & output_flags) {
314-
sprintf(influx->buffer + strlen(influx->buffer),
330+
snprintf(influx->buffer + strlen(influx->buffer),
331+
sizeof(influx->buffer) - strlen(influx->buffer),
315332
"%s,node=%s,host=%s%s%s memoryuse=%lu,pcpu=%d,pmem=%d,readbps=%d,writebps=%d,readiops=%d,writeiops=%d,maxproc=%d,openfiles=%d %lu\n",
316333
influx->tables.bhyve, hostname, sumch->name,
317334
(influx->tags.bhyve == NULL ? "" : ","),
@@ -334,24 +351,21 @@ sum_data_bhyve()
334351
if (OUTPUT_SQLITE3 & output_flags) {
335352
memset(sql, 0, sizeof(sql));
336353
memset(stats_file, 0, sizeof(stats_file));
337-
sprintf(stats_file, "%s/jails-system/%s/racct.sqlite",
354+
snprintf(stats_file, sizeof(stats_file), "%s/jails-system/%s/racct.sqlite",
338355
workdir, sumch->name);
339356
fp = fopen(stats_file, "r");
340357
if (!fp) {
341358
tolog(log_level,
342359
"RACCT not exist, create via updatesql\n");
343-
sprintf(sql,
360+
snprintf(sql, sizeof(sql),
344361
"/usr/local/bin/cbsd /usr/local/cbsd/misc/updatesql %s /usr/local/cbsd/share/racct.schema racct",
345362
stats_file);
346363
system(sql);
347-
// write into base in next loop (protection if
348-
// jail was removed in directory not exist
349-
// anymore
350364
continue;
351365
}
352366
fclose(fp);
353367

354-
sprintf(sql,
368+
snprintf(sql, sizeof(sql),
355369
"INSERT INTO racct ( idx,memoryuse,maxproc,openfiles,pcpu,readbps,writebps,readiops,writeiops,pmem ) VALUES ( '%d', '%lu', '%d', '%d', '%d', '%d', '%d', '%d', '%d', '%d' );\n",
356370
cur_time, sumch->memoryuse / round_total,
357371
sumch->maxproc / round_total,
@@ -381,7 +395,12 @@ sum_data_bhyve()
381395
}
382396

383397
if (OUTPUT_BEANSTALKD & output_flags) {
384-
strcat(json_str, "]}");
398+
if (strlen(json_str) + 2 < sizeof(json_str)) {
399+
strcat(json_str, "]}");
400+
} else {
401+
tolog(log_level, "Buffer overflow in json_str\n");
402+
skip_beanstalk = 1;
403+
}
385404
} else {
386405
skip_beanstalk = 1;
387406
}

‎tools/src/racct-generic-stats.c

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -408,16 +408,14 @@ get_bs_stats(char *yaml, const char *str)
408408
int str_len = 0;
409409
int str_with_val_len = 0;
410410
int yaml_len = 0;
411-
char *tmp;
411+
char *tmp = NULL;
412412
int values = -1;
413413
int i = 0;
414414
int x;
415415
char *token = NULL;
416-
char *tofree;
417416

418417
str_len = strlens(str);
419-
str_with_val_len = str_len +
420-
10; // assume value not greated than: XXXXXXXXXX
418+
str_with_val_len = str_len + 10; // assume value not greater than: XXXXXXXXXX
421419

422420
if (str_len == 0)
423421
return -1;
@@ -434,32 +432,31 @@ get_bs_stats(char *yaml, const char *str)
434432

435433
if (pch) {
436434
tmp = malloc(str_with_val_len);
435+
if (!tmp) {
436+
tolog(log_level, "Failed to allocate memory in get_bs_stats\n");
437+
return -1;
438+
}
439+
memset(tmp, 0, str_with_val_len);
437440
i = 0;
438-
while (pch[i] != '\n') {
441+
while (pch[i] != '\n' && i < str_with_val_len - 1) {
439442
tmp[i] = pch[i];
440443
i++;
441-
if (i >= str_with_val_len)
442-
break;
443444
}
444445
tmp[i] = '\0';
445-
// tolog(log_level,"get_bs_stats: found: [%s]\n",tmp);
446-
x = 0;
447-
tofree = tmp;
448446

447+
x = 0;
449448
while ((token = strsep(&tmp, ":")) != NULL) {
450449
switch (x) {
451450
case 0:
452-
// tolog(log_level,"TOKEN: [%s]\n",token);
453451
break;
454452
case 1:
455-
// tolog(log_level,"TOKEN2: [%s]\n",token);
456453
sscanf(token, "%d", &values);
457454
break;
458455
}
459456
x++;
460457
}
461-
free(tofree);
462458
free(tmp);
459+
tmp = NULL;
463460
} else {
464461
tolog(log_level, "get_bs_stats: no [%s] here\n", str);
465462
}

‎tools/src/racct-hoster-statsd.c

Lines changed: 31 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ sum_data_hoster()
168168
struct item_data *target = NULL;
169169
struct item_data *ch;
170170
struct item_data *next_ch;
171+
const char *hostname = getenv("HOST");
171172
char sql[512];
172173
char stats_file[1024];
173174
int ret = 0;
@@ -189,6 +190,13 @@ sum_data_hoster()
189190
gettimeofday(&now_time, NULL);
190191
cur_time = (time_t)now_time.tv_sec;
191192

193+
// First, free existing sum_item_list
194+
for (sumch = sum_item_list; sumch; sumch = next_sumch) {
195+
next_sumch = sumch->next;
196+
free(sumch);
197+
}
198+
sum_item_list = NULL;
199+
192200
for (ch = item_list; ch; ch = ch->next) {
193201
if (ch->modified == 0) {
194202
continue;
@@ -207,26 +215,22 @@ sum_data_hoster()
207215
sumch->memoryuse += ch->memoryuse;
208216
sumch->maxproc += ch->maxproc;
209217
sumch->openfiles += ch->openfiles;
210-
// sumch->readbps+=ch->readbps;
211-
// sumch->writebps+=ch->writebps;
212-
// sumch->readiops+=ch->readiops;
213-
// sumch->writeiops+=ch->writeiops;
214218
sumch->temperature += ch->temperature;
215219
sumch->pmem += ch->pmem;
216220
break;
217221
}
218222
}
219223
} else {
220224
CREATE(newd, struct sum_item_data, 1);
225+
if (!newd) {
226+
tolog(log_level, "Failed to allocate memory for newd\n");
227+
continue;
228+
}
221229
newd->modified = ch->modified;
222230
newd->pcpu = ch->pcpu;
223231
newd->memoryuse = ch->memoryuse;
224232
newd->maxproc = ch->maxproc;
225233
newd->openfiles = ch->openfiles;
226-
// newd->readbps=ch->readbps;
227-
// newd->writebps=ch->writebps;
228-
// newd->readiops=ch->readiops;
229-
// newd->writeiops=ch->writeiops;
230234
newd->temperature = ch->temperature;
231235
newd->pmem = ch->pmem;
232236
newd->next = sum_item_list;
@@ -249,14 +253,19 @@ sum_data_hoster()
249253
sumch->modified / round_total);
250254
if (OUTPUT_BEANSTALKD & output_flags) {
251255
memset(json_buf, 0, sizeof(json_buf));
252-
sprintf(json_buf,
256+
snprintf(json_buf, sizeof(json_buf),
253257
"{\"name\": \"%s\",\"time\": %d,\"pcpu\": %d,\"pmem\": %d }",
254258
sumch->name, cur_time, sumch->pcpu / round_total,
255259
sumch->pmem / round_total);
256260

257261
if (strlen(json_str) > 2) {
258-
strcat(json_str, ",");
259-
strcat(json_str, json_buf);
262+
if (strlen(json_str) + strlen(json_buf) + 2 < sizeof(json_str)) {
263+
strcat(json_str, ",");
264+
strcat(json_str, json_buf);
265+
} else {
266+
tolog(log_level, "Buffer overflow in json_str\n");
267+
break;
268+
}
260269
} else {
261270
strcpy(json_str,
262271
"{ \"tube\":\"racct-system\", \"node\":\"clonos.convectix.com\", \"data\":[");
@@ -266,8 +275,8 @@ sum_data_hoster()
266275

267276
#ifdef WITH_INFLUX
268277
if (OUTPUT_INFLUX & output_flags) {
269-
270-
sprintf(influx->buffer + strlen(influx->buffer),
278+
snprintf(influx->buffer + strlen(influx->buffer),
279+
sizeof(influx->buffer) - strlen(influx->buffer),
271280
"%s,node=%s,host=%s%s%s memoryuse=%lu,maxproc=%d,openfiles=%d,pcpu=%d,pmem=%d,temperature=%2.2f %lu\n",
272281
influx->tables.nodes, nodename, sumch->name,
273282
(influx->tags.nodes == NULL ? "" : ","),
@@ -280,22 +289,7 @@ sum_data_hoster()
280289
sumch->pmem / round_total,
281290
sumch->temperature / round_total, nanoseconds());
282291

283-
/*
284-
printf("%s,node=%s,host=%s%s%s
285-
memoryuse=%lu,maxproc=%d,openfiles=%d,pcpu=%d,pmem=%d,temperature=%2.2f
286-
%lu\n", influx->tables.nodes, nodename, sumch->name,
287-
(influx->tags.nodes==NULL?"":","),
288-
(influx->tags.nodes==NULL?"":influx->tags.nodes),
289-
sumch->memoryuse/round_total,
290-
sumch->maxproc/round_total,
291-
sumch->openfiles/round_total,
292-
sumch->pcpu/round_total,
293-
sumch->pmem/round_total,sumch->temperature/round_total,
294-
nanoseconds());
295-
*/
296292
influx->items++;
297-
// tolog(log_level,"%d RACCT items
298-
// queued for storage\n", influx->items);
299293
}
300294
#endif
301295
#ifdef WITH_REDIS
@@ -305,24 +299,21 @@ sum_data_hoster()
305299
if (OUTPUT_SQLITE3 & output_flags) {
306300
memset(sql, 0, sizeof(sql));
307301
memset(stats_file, 0, sizeof(stats_file));
308-
sprintf(stats_file, "%s/jails-system/%s/racct.sqlite",
302+
snprintf(stats_file, sizeof(stats_file), "%s/jails-system/%s/racct.sqlite",
309303
workdir, sumch->name);
310304
fp = fopen(stats_file, "r");
311305
if (!fp) {
312306
tolog(log_level,
313307
"RACCT not exist, create via updatesql\n");
314-
sprintf(sql,
308+
snprintf(sql, sizeof(sql),
315309
"/usr/local/bin/cbsd /usr/local/cbsd/misc/updatesql %s /usr/local/cbsd/share/racct.schema racct",
316310
stats_file);
317311
system(sql);
318-
// write into base in next loop (protection if
319-
// jail was removed in directory not exist
320-
// anymore
321312
continue;
322313
}
323314
fclose(fp);
324315

325-
sprintf(sql,
316+
snprintf(sql, sizeof(sql),
326317
"INSERT INTO racct ( idx,memoryuse,maxproc,openfiles,pcpu,pmem ) VALUES ( '%d', '%lu', '%d', '%d', '%d', '%d' );\n",
327318
cur_time, sumch->memoryuse / round_total,
328319
sumch->maxproc / round_total,
@@ -338,18 +329,19 @@ sum_data_hoster()
338329
sumch->memoryuse = 0;
339330
sumch->maxproc = 0;
340331
sumch->openfiles = 0;
341-
// sumch->readbps=0;
342-
// sumch->writebps=0;
343-
// sumch->readiops=0;
344-
// sumch->writeiops=0;
345332
sumch->temperature = 0;
346333
sumch->pmem = 0;
347334

348335
remove_data_by_jname(sumch->name);
349336
}
350337

351338
if (OUTPUT_BEANSTALKD & output_flags) {
352-
strcat(json_str, "]}");
339+
if (strlen(json_str) + 2 < sizeof(json_str)) {
340+
strcat(json_str, "]}");
341+
} else {
342+
tolog(log_level, "Buffer overflow in json_str\n");
343+
skip_beanstalk = 1;
344+
}
353345
bs_tick = 0;
354346
}
355347

0 commit comments

Comments
 (0)