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 a3d9f8ae49270ea8b8c0ce53c6fe2accc99fc59f..83e707a2ad10bb4239318836e16eb0bc209b3ca4 100644 (file)
--- 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)));
                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;
                        return NULL;

+               }

                tl->next = traces;
                tl->name = (char *)(tl + 1);
                snprintf(tl->name, n, "%s/%s", dir_name, d->d_name);
                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 2b9667c34d8514eb6ae0a0680e51e5b7e0c2bdbf..cbf6c9adf622d4076a1eebcafe46b0a346290e8d 100644 (file)
--- a/iowatcher/main.c
+++ b/iowatcher/main.c
@@ -713,6 +713,22 @@ static int count_io_plot_types(void)
        return total_io_types;
 }
 
        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)
 {
 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) {
        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]);
 
                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 (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)
                }
        }
        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) {
                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) {
 
                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++) {
                        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;
                        }
 
                        batch_i = 0;