iowatcher: Fix up some strcpy and strcat usage
Fix an unchecked strcpy and strcat in plot_io_movie():
$ ./iowatcher -t foo --movie -o foo.ogv -l $(printf 'x%.0s' {1..300})
[...]
*** buffer overflow detected ***: ./iowatcher terminated
There was also very similar code in plot_io() so a new function
plot_io_legend() was added to factor out the common string building code
and replace the buggy code with asprintf().
Also add a closedir() call to an error path in traces_list() to plug a
resource leak and make iowatcher Coverity-clean (ignoring some
false-positives).
Signed-off-by: Andrew Price <anprice@redhat.com>
diff --git a/iowatcher/blkparse.c b/iowatcher/blkparse.c
index a3d9f8a..83e707a 100644
--- a/iowatcher/blkparse.c
+++ b/iowatcher/blkparse.c
@@ -751,8 +751,10 @@
n += dlen + 1; /* dir + '/' + file */
/* Allocate space for tracelist + filename */
tl = calloc(1, sizeof(struct tracelist) + (sizeof(char) * (n + 1)));
- if (!tl)
+ if (!tl) {
+ closedir(dir);
return NULL;
+ }
tl->next = traces;
tl->name = (char *)(tl + 1);
snprintf(tl->name, n, "%s/%s", dir_name, d->d_name);
diff --git a/iowatcher/main.c b/iowatcher/main.c
index 2b9667c..cbf6c9a 100644
--- a/iowatcher/main.c
+++ b/iowatcher/main.c
@@ -713,6 +713,22 @@
return total_io_types;
}
+static void plot_io_legend(struct plot *plot, struct graph_dot_data *gdd, char *prefix, char *rw)
+{
+ int ret = 0;
+ char *label = NULL;
+ if (io_per_process)
+ ret = asprintf(&label, "%s %s", prefix, gdd->label);
+ else
+ ret = asprintf(&label, "%s", prefix);
+ if (ret < 0) {
+ perror("Failed to process labels");
+ exit(1);
+ }
+ svg_add_legend(plot, label, rw, gdd->color);
+ free(label);
+}
+
static void plot_io(struct plot *plot, unsigned int min_seconds,
unsigned int max_seconds, u64 min_offset, u64 max_offset)
{
@@ -733,33 +749,17 @@
set_xticks(plot, num_xticks, min_seconds, max_seconds);
list_for_each_entry(tf, &all_traces, list) {
- char label[256];
- char *pos;
-
- if (!tf->label)
- label[0] = 0;
- else {
- strncpy(label, tf->label, 255);
- label[255] = 0;
- if (io_per_process)
- strcat(label, " ");
- }
- pos = label + strlen(label);
+ char *prefix = tf->label ? tf->label : "";
for (i = 0; i < tf->io_plots; i++) {
if (tf->gdd_writes[i]) {
svg_io_graph(plot, tf->gdd_writes[i]);
- if (io_per_process)
- strcpy(pos, tf->gdd_writes[i]->label);
- svg_add_legend(plot, label, " Writes", tf->gdd_writes[i]->color);
+ plot_io_legend(plot, tf->gdd_writes[i], prefix, " Writes");
}
if (tf->gdd_reads[i]) {
svg_io_graph(plot, tf->gdd_reads[i]);
- if (io_per_process)
- strcpy(pos, tf->gdd_reads[i]->label);
- svg_add_legend(plot, label, " Reads", tf->gdd_reads[i]->color);
+ plot_io_legend(plot, tf->gdd_reads[i], prefix, " Reads");
}
-
}
}
if (plot->add_xlabel)
@@ -1093,17 +1093,7 @@
batch_count = 1;
list_for_each_entry(tf, &all_traces, list) {
- char label[256];
- char *pos;
-
- if (!tf->label)
- label[0] = 0;
- else {
- strcpy(label, tf->label);
- if (io_per_process)
- strcat(label, " ");
- }
- pos = label + strlen(label);
+ char *prefix = tf->label ? tf->label : "";
i = 0;
while (i < cols) {
@@ -1143,16 +1133,10 @@
history->col = i;
for (pid = 0; pid < tf->io_plots; pid++) {
- if (tf->gdd_reads[pid]) {
- if (io_per_process)
- strcpy(pos, tf->gdd_reads[pid]->label);
- svg_add_legend(plot, label, " Reads", tf->gdd_reads[pid]->color);
- }
- if (tf->gdd_writes[pid]) {
- if (io_per_process)
- strcpy(pos, tf->gdd_writes[pid]->label);
- svg_add_legend(plot, label, " Writes", tf->gdd_writes[pid]->color);
- }
+ if (tf->gdd_reads[pid])
+ plot_io_legend(plot, tf->gdd_reads[pid], prefix, " Reads");
+ if (tf->gdd_writes[pid])
+ plot_io_legend(plot, tf->gdd_writes[pid], prefix, " Writes");
}
batch_i = 0;
