[PC-BSD Testing] acl_from_text leaking memory

[doverosx at gmail.com]

Jim Wilcoxson wrote:
> I've been working on a new backup program, HashBackup, and believe I
> have found a memory leak with ACLs in PCBSD/FreeBSD 7.1 and OSX
> (Leopard).
>
> acl_from_text is a function that takes a text string as input, and
> returns a pointer to a malloc'd acl.  This acl is then freed with
> acl_free.  I noticed that acl_from_text appears to leak memory.  This
> is not used during the backup of a filesystem, but is needed to do a
> restore.
>
> After looking at the acl_from_text source in /usr/src/lib/libc/posix1e
> (from PCBSD7.1), I believe the problem is that the duplicate text
> string, mybuf_p, is not freed on normal return of this function.  Here
> is the end of this function:
>
>         }
>
> #if 0
>         /* XXX Should we only return ACLs valid according to acl_valid? */
>         /* Verify validity of the ACL we read in. */
>         if (acl_valid(acl) == -1) {
>                 errno = EINVAL;
>                 goto error_label;
>         }
> #endif
>
>         return(acl);
>
> error_label:
>         acl_free(acl);
>         free(mybuf_p);
>         return(NULL);
> }
>
> I think there should be a free(mybuf_p) before return(acl).
>
> Here is a PCBSD/FreeBSD test program that causes the memory leak:
>
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/acl.h>
>
> main() {
>    acl_t acl;
>    char* acltext;
>
>    acltext = "user::rw-\n group::r--\n mask::r--\n other::r--\n";
>    while (1) {
>       acl = acl_from_text(acltext);
>       if (acl == NULL)
>         printf("acl_from_text failed\n");
>       if (acl_free(acl) != 0)
>         printf("acl_free failed\n");
>   }
> }
>
> I've subscribed to the lists for a few days in case there are
> questions or I can help test something.
>
> Thanks,
> Jim
> --
> HashBackup beta: http://sites.google.com/site/hashbackup
> _______________________________________________
> Testing mailing list
> Testing at lists.pcbsd.org
> http://lists.pcbsd.org/mailman/listinfo/testing
>
>   
What is the mybuf_p structure? From what I can interpret in the source, 
mybuf_p is likely to be kept because our acl is indeed valid. Perhaps 
mybuf_p should be freed in a more logical function?

Regards,
Brodey