Bug 6007 - regtree tool has lots of valgrind errors
Summary: regtree tool has lots of valgrind errors
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: Other (show other bugs)
Version: unspecified
Hardware: x86 Linux
: P3 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-02 01:06 UTC by Michael
Modified: 2013-07-03 03:25 UTC (History)
1 user (show)

See Also:


Attachments
Possible patch bugfix (3.09 KB, patch)
2009-01-02 05:16 UTC, Michael
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael 2009-01-02 01:06:28 UTC
valgrind -v --leak-check=full --show-reachable=yes  ./bin/regtree --file /tmp/ntuser.dat  > /dev/null

produces uninitialised read errors, and memory leaks:

==3650== 416 errors in context 6 of 11:
==3650== Invalid read of size 1
==3650==    at 0x84F8BDF: data_blob_hex_string (data_blob.c:167)
==3650==    by 0x80B251C: reg_val_data_string (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B25B5: reg_val_description (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C00: print_tree (regtree.c:83)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B1066: main (regtree.c:149)
==3650==  Address 0x4a86d54 is 60 bytes inside a block of size 76 free'd
==3650==    at 0x402265C: free (vg_replace_malloc.c:323)
==3650==    by 0x85098A9: _talloc_free (talloc.c:577)
==3650==    by 0x8509D82: talloc_free (talloc.c:923)
==3650==    by 0x80B7ABD: regf_get_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BE946: hive_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BF2A7: local_enum_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B1E96: reg_key_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C41: print_tree (regtree.c:78)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B1066: main (regtree.c:149)
==3650== 
==3650== 1210 errors in context 7 of 11:
==3650== Invalid read of size 1
==3650==    at 0x84FF905: utf8_push (iconv.c:612)
==3650==    by 0x84FE90D: smb_iconv (iconv.c:129)
==3650==    by 0x85002E7: iconv_talloc (charcnv.c:200)
==3650==    by 0x8500785: convert_string_talloc_convenience (charcnv.c:327)
==3650==    by 0x80B2502: reg_val_data_string (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B25B5: reg_val_description (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C00: print_tree (regtree.c:83)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==  Address 0x461f9c4 is 60 bytes inside a block of size 76 free'd
==3650==    at 0x402265C: free (vg_replace_malloc.c:323)
==3650==    by 0x85098A9: _talloc_free (talloc.c:577)
==3650==    by 0x8509D82: talloc_free (talloc.c:923)
==3650==    by 0x80B7ABD: regf_get_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BE946: hive_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BF2A7: local_enum_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B1E96: reg_key_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C41: print_tree (regtree.c:78)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650== 
==3650== 1210 errors in context 8 of 11:
==3650== Invalid read of size 1
==3650==    at 0x84FF8FB: utf8_push (iconv.c:610)
==3650==    by 0x84FE90D: smb_iconv (iconv.c:129)
==3650==    by 0x85002E7: iconv_talloc (charcnv.c:200)
==3650==    by 0x8500785: convert_string_talloc_convenience (charcnv.c:327)
==3650==    by 0x80B2502: reg_val_data_string (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B25B5: reg_val_description (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C00: print_tree (regtree.c:83)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==  Address 0x461f9c4 is 60 bytes inside a block of size 76 free'd
==3650==    at 0x402265C: free (vg_replace_malloc.c:323)
==3650==    by 0x85098A9: _talloc_free (talloc.c:577)
==3650==    by 0x8509D82: talloc_free (talloc.c:923)
==3650==    by 0x80B7ABD: regf_get_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BE946: hive_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BF2A7: local_enum_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B1E96: reg_key_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C41: print_tree (regtree.c:78)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650== 
==3650== 1210 errors in context 9 of 11:
==3650== Invalid read of size 1
==3650==    at 0x84FF8F1: utf8_push (iconv.c:610)
==3650==    by 0x84FE90D: smb_iconv (iconv.c:129)
==3650==    by 0x85002E7: iconv_talloc (charcnv.c:200)
==3650==    by 0x8500785: convert_string_talloc_convenience (charcnv.c:327)
==3650==    by 0x80B2502: reg_val_data_string (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B25B5: reg_val_description (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C00: print_tree (regtree.c:83)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==  Address 0x461f9c5 is 61 bytes inside a block of size 76 free'd
==3650==    at 0x402265C: free (vg_replace_malloc.c:323)
==3650==    by 0x85098A9: _talloc_free (talloc.c:577)
==3650==    by 0x8509D82: talloc_free (talloc.c:923)
==3650==    by 0x80B7ABD: regf_get_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BE946: hive_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BF2A7: local_enum_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B1E96: reg_key_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C41: print_tree (regtree.c:78)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650== 
==3650== 3291 errors in context 10 of 11:
==3650== Invalid read of size 4
==3650==    at 0x80B254E: reg_val_data_string (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B25B5: reg_val_description (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C00: print_tree (regtree.c:83)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B1066: main (regtree.c:149)
==3650==  Address 0x4764bcc is 60 bytes inside a block of size 76 free'd
==3650==    at 0x402265C: free (vg_replace_malloc.c:323)
==3650==    by 0x85098A9: _talloc_free (talloc.c:577)
==3650==    by 0x8509D82: talloc_free (talloc.c:923)
==3650==    by 0x80B7ABD: regf_get_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BE946: hive_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BF2A7: local_enum_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B1E96: reg_key_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C41: print_tree (regtree.c:78)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B1066: main (regtree.c:149)
==3650== 
==3650== 4445 errors in context 11 of 11:
==3650== Invalid read of size 4
==3650==    at 0x80B252B: reg_val_data_string (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B25B5: reg_val_description (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C00: print_tree (regtree.c:83)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B1066: main (regtree.c:149)
==3650==  Address 0x476481c is 60 bytes inside a block of size 76 free'd
==3650==    at 0x402265C: free (vg_replace_malloc.c:323)
==3650==    by 0x85098A9: _talloc_free (talloc.c:577)
==3650==    by 0x8509D82: talloc_free (talloc.c:923)
==3650==    by 0x80B7ABD: regf_get_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BE946: hive_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80BF2A7: local_enum_value (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B1E96: reg_key_get_value_by_index (in /var/tmp/build/samba/source4/bin/regtree)
==3650==    by 0x80B0C41: print_tree (regtree.c:78)
==3650==    by 0x80B0AAD: print_tree (regtree.c:64)
==3650==    by 0x80B1066: main (regtree.c:149)


And also there are many memory leaks. I added a         talloc_enable_leak_report_full(); and I get an output like:

full talloc report on 'null_context' (total 4197123 bytes in 91805 blocks)
    struct registry_local          contains 1833463 bytes in 91305 blocks (ref 1608) 0x85fb028
        struct local_key               contains    207 bytes in  10 blocks (ref 0) 0x8daea08
            reference to: struct registry_local
            struct regf_key_data           contains    179 bytes in   8 blocks (ref 0) 0x8daecc0
                const char *                   contains     33 bytes in   3 blocks (ref 0) 0x8daea50
                    IniFiles                       contains      9 bytes in   1 blocks (ref 0) 0x8dae9c8
                    reference to: Windows 3.1 Migration Status
                struct nk_block                contains    118 bytes in   3 blocks (ref 0) 0x8daebf0
                    char                           contains     26 bytes in   1 blocks (ref 0) 0x8daed90
                    char                           contains      8 bytes in   1 blocks (ref 0) 0x8daed50
                reference to: struct regf_data
        struct local_key               contains    199 bytes in  10 blocks (ref 0) 0x8dae7f0
            reference to: struct registry_local
            struct regf_key_data           contains    171 bytes in   8 blocks (ref 0) 0x8dae320
                const char *                   contains     31 bytes in   3 blocks (ref 0) 0x8dae590
                    Groups                         contains      7 bytes in   1 blocks (ref 0) 0x8dae480
                    reference to: Windows 3.1 Migration Status
                struct nk_block                contains    112 bytes in   3 blocks (ref 0) 0x8dae768
                    char                           contains     20 bytes in   1 blocks (ref 0) 0x8dae8b8
                    char                           contains      8 bytes in   1 blocks (ref 0) 0x8dae878
                reference to: struct regf_data

.... and this keeps going (looks like all struct local_key allocations remain attached to struct registry_local without being freed).
Comment 1 Michael 2009-01-02 04:04:06 UTC
Hi,
  After more investigation I think I found the source of the uninitialised reads:

In regf.c around line 547 there is code like

data->data = (uint8_t *)&vk->data_offset; 
and then talloc_free(vk) towards the end of the function
data is passed back to the caller with a reference to memory which got freed
this is then used in the call to iconv

This is a possible patch to fix the issue, which certainly stops the valgrind warnings:

 if (vk->data_length & 0x80000000) { 
                vk->data_length &=~0x80000000; 
                data->data = talloc_size(ctx, vk->data_length); 
                memcpy(data->data, (uint8_t *)&vk->data_offset, vk->data_length); 
                data->length = vk->data_length; 
                //data->data = (uint8_t *)&vk->data_offset; 
                //data->length = 0; 
        } else { 
                *data = hbin_get(regf, vk->data_offset); 
        } 
Comment 2 Michael 2009-01-02 05:16:09 UTC
Created attachment 3844 [details]
Possible patch bugfix

The main memory leak was narrowed down to a missing 
               talloc_free(subkey);
in the print_tree function - the function used reg_open_key() to open each new subkey but neglected to free the subkey on each iteration. This could cause an accumulation of leaks until the main context was freed.

There was also a missing context in reg_common_open_file which allocated things to NULL - this makes it impossible to free and leads to a leak. Its ok if the registry file is meant to be permanently resident.
Comment 3 Matthias Dieter Wallnöfer 2009-01-03 07:49:12 UTC
Good, should be fixed (patches in GIT branch). Thanks for reporting this!