From 177d648bdd959ad4de02ca07aece5e6053682816 Mon Sep 17 00:00:00 2001
From: Andrew Price <anprice@redhat.com>
Date: Sun, 27 Apr 2014 06:16:01 +0100
Subject: [PATCH] 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>
---
 iowatcher/blkparse.c |  4 ++-
 iowatcher/main.c     | 64 +++++++++++++++++---------------------------
 2 files changed, 27 insertions(+), 41 deletions(-)

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 @@ static struct tracelist *traces_list(char *dir_name, int *len)
 		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 @@ static int count_io_plot_types(void)
 	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 @@ static void plot_io(struct plot *plot, unsigned int min_seconds,
 	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 @@ static void plot_io_movie(struct plot *plot)
 		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 @@ static void plot_io_movie(struct plot *plot)
 			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;
-- 
2.25.1