[PC-BSD Testing] acl_from_text leaking memory

[Jim Wilcoxson]

mybuf_p is a temporary copy of the text acl.  The reason a temp is
created is so that while parsing the acl text with strsep, it can be
modified by inserting zero bytes.  There are actually 2 mallocs
happening: one is the temporary text string, mybuf_p; the other is the
acl itself that will be returned.  You're right - it would not do to
free the acl itself.  The caller of acl_from_text has to do that with
acl_free, and the test program I used does this.

The leak is (I think) caused by the temporary copy of the text string:
it isn't freed after the acl has been parsed.  I would test this by
making the change and creating a new kernel, but I've never built a
BSD kernel.

Jim

On 11/15/09, doverosx at gmail.com <doverosx at gmail.com> wrote:
> 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
> _______________________________________________
> Testing mailing list
> Testing at lists.pcbsd.org
> http://lists.pcbsd.org/mailman/listinfo/testing
>