Re: Invalid memory read in FSFS

Philip Martin <philip.martin_at_wandisco.com> writes:

> I'm seeing test failures when building Subversion and APR with pool
> debugging:
>
> FAIL: lt-fs-fs-pack-test 13: upgrade txns to log addressing in shared FSFS
> FAIL: lt-fs-fs-pack-test 14: upgrade txns started before svnadmin upgrade
> FAIL: lt-fs-test 33: test dir prop preservation in unordered txns
> FAIL: lt-fs-test 43: test merge directory properties
>
> Valgrind reports are typically:
>
> $ valgrind -q .libs/lt-fs-test --cleanup 33
> ==32655== Invalid read of size 1
> ==32655== at 0x402EA79: bcmp (mc_replace_strmem.c:935)
> ==32655== by 0x41F6E72: find_entry (apr_hash.c:280)
> ==32655== by 0x41F719D: apr_hash_set (apr_hash.c:357)
> ==32655== by 0x50430CB: read_dir_entries (cached_data.c:2053)
> ==32655== by 0x50432A4: get_dir_contents (cached_data.c:2098)
> ==32655== by 0x5043640: svn_fs_fs__rep_contents_dir (cached_data.c:2195)
> ==32655== by 0x50714B3: write_final_rev (transaction.c:2897)
> ==32655== by 0x5074113: commit_body (transaction.c:3839)
> ==32655== by 0x504A898: with_some_lock_file (fs_fs.c:185)
> ==32655== by 0x504A970: svn_fs_fs__with_write_lock (fs_fs.c:203)
> ==32655== by 0x5074BB1: svn_fs_fs__commit (transaction.c:4009)
> ==32655== by 0x507983F: svn_fs_fs__commit_txn (tree.c:2228)
> ==32655== Address 0x870d120 is 0 bytes inside a block of size 2 free'd
> ==32655== at 0x402AF4C: free (vg_replace_malloc.c:468)
> ==32655== by 0x42021F9: pool_clear_debug (apr_pools.c:1576)
> ==32655== by 0x42022D9: apr_pool_clear_debug (apr_pools.c:1613)
> ==32655== by 0x5042DCC: read_dir_entries (cached_data.c:1993)
> ==32655== by 0x50432A4: get_dir_contents (cached_data.c:2098)
> ==32655== by 0x5043640: svn_fs_fs__rep_contents_dir (cached_data.c:2195)
> ==32655== by 0x50714B3: write_final_rev (transaction.c:2897)
> ==32655== by 0x5074113: commit_body (transaction.c:3839)
> ==32655== by 0x504A898: with_some_lock_file (fs_fs.c:185)
> ==32655== by 0x504A970: svn_fs_fs__with_write_lock (fs_fs.c:203)
> ==32655== by 0x5074BB1: svn_fs_fs__commit (transaction.c:4009)
> ==32655== by 0x507983F: svn_fs_fs__commit_txn (tree.c:2228)
>
> I think this was caused by r1554711, switch directory rep to array
> order, but I haven't identified the exact problem yet.

I think it might be:

Index: sw/subversion/src/subversion/libsvn_fs_fs/cached_data.c
===================================================================
--- sw/subversion/src/subversion/libsvn_fs_fs/cached_data.c (revision 1571876)
+++ sw/subversion/src/subversion/libsvn_fs_fs/cached_data.c (working copy)
@@ -2050,7 +2050,10 @@
       /* In incremental mode, update the hash; otherwise, write to the
        * final array. */
       if (incremental)
- apr_hash_set(hash, entry.key, entry.keylen, dirent);
+ apr_hash_set(hash,
+ apr_pstrmemdup(scratch_pool, entry.key, entry.keylen),
+ entry.keylen,
+ dirent);
       else
         APR_ARRAY_PUSH(entries, svn_fs_dirent_t *) = dirent;
     }

that removes the above valgrind warning but now I see:

$ valgrind -q .libs/lt-fs-test --cleanup 33
==26848== Invalid read of size 8
==26848== at 0x50513AE: svn_fs_fs__id_txn_used (id.c:151)
==26848== by 0x505CB34: svn_fs_fs__unparse_representation (low_level.c:890)
==26848== by 0x505CE8A: svn_fs_fs__write_noderev (low_level.c:939)
==26848== by 0x506B76A: svn_fs_fs__put_node_revision (transaction.c:499)
==26848== by 0x50497AC: svn_fs_fs__dag_update_ancestry (dag.c:1346)
==26848== by 0x5079474: merge (tree.c:2070)
==26848== by 0x50791B4: merge (tree.c:2021)
==26848== by 0x50791B4: merge (tree.c:2021)
==26848== by 0x507966A: merge_changes (tree.c:2121)
==26848== by 0x5079800: svn_fs_fs__commit_txn (tree.c:2218)
==26848== by 0x4048248: svn_fs_commit_txn (fs-loader.c:830)
==26848== by 0x402CAB: test_commit_txn (fs-test.c:81)
==26848== Address 0x7775668 is 72 bytes inside a block of size 112 free'd
==26848== at 0x402AF4C: free (vg_replace_malloc.c:468)
==26848== by 0x42021F9: pool_clear_debug (apr_pools.c:1576)
==26848== by 0x42022D9: apr_pool_clear_debug (apr_pools.c:1613)
==26848== by 0x5078B3A: merge (tree.c:1916)
==26848== by 0x50791B4: merge (tree.c:2021)
==26848== by 0x50791B4: merge (tree.c:2021)
==26848== by 0x507966A: merge_changes (tree.c:2121)
==26848== by 0x5079800: svn_fs_fs__commit_txn (tree.c:2218)
==26848== by 0x4048248: svn_fs_commit_txn (fs-loader.c:830)
==26848== by 0x402CAB: test_commit_txn (fs-test.c:81)
==26848== by 0x418544: unordered_txn_dirprops (fs-test.c:4655)
==26848== by 0x403BD93: do_test_num (svn_test_main.c:400)

-- 
Philip