tail: -r mem leak with non-regular files

[Tobias Stoeckmann]

Sent this back in March, so maybe someone wants to review this time? :)

tail -r has two memory leaks when handling non-regular files. You can
easily see memory usage increasing with commands like

$ mknod pipe p
$ tail -r pipe pipe pipe > /dev/null

And then writing a large file into the pipe three times. top shows the
memory usage of tail increasing with each consecutive read.

The obvious memory leak is at the end of reverse(): the doubly linked
list (actually circular list) is never freed. To fix this, I break the
circle and iterate through it, freeing items until I hit NULL.

The less obvious one is in an error path. As tl->l is always of fixed
size (BSZ), we can just change the struct to have a BSZ sized array in
it. This removes the need to do checks in the error path completely.

While at it, there is no use to have a typedef in the code, so I just
removed it, too.


Tobias

Index: reverse.c
===================================================================
RCS file: /cvs/src/usr.bin/tail/reverse.c,v
retrieving revision 1.19
diff -u -p -r1.19 reverse.c
--- reverse.c   27 Oct 2009 23:59:44 -0000      1.19
+++ reverse.c   12 Jul 2015 12:34:21 -0000
@@ -147,12 +147,13 @@ r_reg(FILE *fp, enum STYLE style, off_t 
        return (0);
 }
 
-typedef struct bf {
+#define        BSZ     (128 * 1024)
+struct bf {
        struct bf *next;
        struct bf *prev;
        size_t len;
-       char *l;
-} BF;
+       char l[BSZ];
+};
 
 /*
  * r_buf -- display a non-regular file in reverse order by line.
@@ -167,21 +168,19 @@ typedef struct bf {
 static void
 r_buf(FILE *fp)
 {
-       BF *mark, *tr, *tl = NULL;
+       struct bf *mark, *tr, *tl = NULL;
        int ch;
        size_t len, llen;
        char *p;
        off_t enomem;
 
-#define        BSZ     (128 * 1024)
        for (mark = NULL, enomem = 0;;) {
                /*
                 * Allocate a new block and link it into place in a doubly
                 * linked list.  If out of memory, toss the LRU block and
                 * keep going.
                 */
-               if (enomem || (tl = malloc(sizeof(BF))) == NULL ||
-                   (tl->l = malloc(BSZ)) == NULL) {
+               if (enomem || (tl = malloc(sizeof(*tl))) == NULL) {
                        if (!mark)
                                err(1, NULL);
                        tl = enomem ? tl->next : mark;
@@ -259,5 +258,12 @@ r_buf(FILE *fp)
        while ((tl = tl->next)->len) {
                WR(tl->l, tl->len);
                tl->len = 0;
+       }
+
+       tl->prev->next = NULL;
+       while (tl != NULL) {
+               tr = tl->next;
+               free(tl);
+               tl = tr;
        }
 }