Re: libattr - severe memory leaks from attr_copy_file()

[Zdenek Prikryl <zprikryl@xxxxxxxxxx>]

> And the variable, text, is assigned straight away before any use,
> so I missed where the problem is.
>
> --Tim

The memory leak is really there. Look:

54 attr_parse_attr_conf(struct error_context *ctx)
...
66 repeat:
67         text = malloc(size_guess + 1);
68         if (!text)
69                 goto fail;
70
71         if ((file = fopen(ATTR_CONF, "r")) == NULL) {
72                 if (errno == ENOENT)
73                         return 0;
74                 goto fail;
75         }

Let's say that malloc() on the line 67 success, so we have text != NULL. Then,
fopen() on the line 71 fails and errno == ENOENT. In that case
attr_parse_attr_conf() simply returns 0, but text isn't freed. That's the point,
where memory leaks arise. I rewrote the patch, so now is more simpler.

-- 
Zdenek Prikryl <zprikryl@xxxxxxxxxx>

diff -up attr-2.4.43/libattr/attr_copy_action.c.leak 
attr-2.4.43/libattr/attr_copy_action.c
--- attr-2.4.43/libattr/attr_copy_action.c.leak 2008-06-30 07:22:50.000000000 
+0200
+++ attr-2.4.43/libattr/attr_copy_action.c      2009-02-17 09:50:38.000000000 
+0100
@@ -53,7 +53,7 @@ free_attr_actions(void)
 static int
 attr_parse_attr_conf(struct error_context *ctx)
 {
-       char *text, *t;
+       char *text = NULL, *t;
        size_t size_guess = 4096, len;
        FILE *file;
        char *pattern = NULL;
@@ -64,15 +64,16 @@ attr_parse_attr_conf(struct error_contex
                return 0;
 
 repeat:
-       text = malloc(size_guess + 1);
-       if (!text)
-               goto fail;
-
        if ((file = fopen(ATTR_CONF, "r")) == NULL) {
                if (errno == ENOENT)
                        return 0;
                goto fail;
        }
+
+       text = malloc(size_guess + 1);
+       if (!text)
+               goto fail;
+
        len = fread(text, 1, size_guess, file);
        if (ferror(file))
                goto fail;