rsync - Sep 2011 - [Bug 8475] New: memory leak around free_xattr() and rsync_xal_free(), with -aX, 200 bytes per file and per directory

https://bugzilla.samba.org/show_bug.cgi?id=8475

           Summary: memory leak around free_xattr() and rsync_xal_free(),
                    with -aX, 200 bytes per file and per directory
           Product: rsync
           Version: 3.0.9
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: core
        AssignedTo: wayned at samba.org
        ReportedBy: tim.liim at alcatel-lucent.com
         QAContact: rsync-qa at samba.org


Description of problem:
    With flag "-aX", rsync creates xattr data structure, but does not
free
    it when done.  This leads to 200 bytes of memory leak for each file
    and each dir, or a few giga for my big backup job.

    In free_xattr(), we have
        rsync_xal_free(sxp->xattr);
        free(sxp->xattr);
        sxp->xattr = NULL;
    In rsync_xal_free() we should also free the pointer to allocated memory:
        rsync_xa *rxas = xalp->items;  // "xalp" is
"sxp->xattr"
        free(rxas);            <== need to add this line
        xalp->items = NULL;    <== for good practice.
        xalp->count = 0;
    because xalp->items comes from malloc() earlier in expand_item_list():
        new_ptr = _realloc_array(...); // wrapper of malloc() and realloc()
        lp->items = new_ptr;

Version-Release number of selected component (if applicable):
    rsync-3.0.9pre2.tar.gz
    rsync-3.0.8-1.fc14.x86_64

How reproducible:
    always, with "-aX" flag.

Steps to Reproduce:
1. create a few source files
    d1=/tmp/t1
    d2=/tmp/t2
    mkdir $d1
    cd    $d1
    for i in {1..40}; do mkdir -p $i; echo 123 > $i/a$i; cp $i/a$i $i/b$i;
done

2. use valgrind to check for memory leak
    rm -rf $d2; 
    valgrind --leak-check=full rsync -aX $d1/ $d2/

Actual results:
    valgrind indicates memory leak.

Expected results:
    no memory leak, at least no big ones.

Additional info:
  - valgrind reports 200 bytes leak for each file processed:
    16,000 bytes in 80 blocks are definitely lost in loss record 27 of 30
       at 0x4A05E46: malloc (vg_replace_malloc.c:195)
       by 0x41DFD7: _realloc_array (util.c:1438)
       by 0x41EC36: expand_item_list (util.c:1690)
       by 0x43C3DE: rsync_xal_get (xattrs.c:268)
       by 0x43C602: get_xattr (xattrs.c:309)
       by 0x4076EF: send_file_name (flist.c:1433)
       by 0x40817D: send_directory (flist.c:1673)
       by 0x408991: send1extra (flist.c:1827)
       by 0x408D23: send_extra_file_list (flist.c:1899)
       by 0x417345: send_files (sender.c:184)
       by 0x42110A: client_run (main.c:1039)
       by 0x421B9B: start_client (main.c:1287)

  - There is another unrelated leak at much lower rate; 
    109 bytes in 39 blocks are definitely lost in loss record 17 of 30
       at 0x4A05E46: malloc (vg_replace_malloc.c:195)
       by 0x41DF8F: _new_array (util.c:1430)
       by 0x406D00: make_file (flist.c:1215)
       by 0x40725B: send_file_name (flist.c:1333)
       by 0x40817D: send_directory (flist.c:1673)
       by 0x408991: send1extra (flist.c:1827)
       by 0x408D23: send_extra_file_list (flist.c:1899)
       by 0x417345: send_files (sender.c:184)
       by 0x42110A: client_run (main.c:1039)
       by 0x421B9B: start_client (main.c:1287)
       by 0x4221E0: main (main.c:1514)
   Just FYI; not sure if you want to fix this one.

-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.

https://bugzilla.samba.org/show_bug.cgi?id=8475

--- Comment #1 from Tim Taiwanese Liim <tim.liim at alcatel-lucent.com>
2011-09-22 02:00:20 UTC ---
This issue was oroginally reported at
    https://bugzilla.redhat.com/show_bug.cgi?id=739794
    memory leak around free_xattr() and rsync_xal_free(), 200 bytes
    per file and per directory

-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.

https://bugzilla.samba.org/show_bug.cgi?id=8475

Wayne Davison <wayned at samba.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #2 from Wayne Davison <wayned at samba.org> 2011-09-22
16:35:42 UTC ---
Looks like rsync_xal_free() was originally written to allow re-use of the
allocated memory (it leaves the malloced value unzeroed along with the
"items"
memory), but only one of the 2 callers was freeing the item memory, and neither
was expecting it to stick around.

I have committed a fix that frees the memory and also avoids calling free on
the "items" list if no allocations were done.  I decided to get rid of
the
memory clearing, since no callers expect to re-use the allocated area -- one
discards the memory, and the other pre-clears it (using EMPTY_ITEM_LIST) before
each new/reused item is used.

-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.

https://bugzilla.samba.org/show_bug.cgi?id=8475

--- Comment #3 from Tim Taiwanese Liim <tim.liim at alcatel-lucent.com>
2011-09-23 15:07:10 UTC ---
Wayne,
Thanks for fast action!  In which release will we see this change?

-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.