Sergey Raevskiy <sergey.raevskiy_at_visualsvn.com> writes:
> This happens beacuse lb.infos field is getting initialized only in function
> lock_body() (see the code below). So, if svn_fs_fs__with_write_lock() fails
> without actual invoking the lock_body(), lb.infos will be left uninitialized.
[...]
> I've attached the patch with crashing test and simple fix for this issue.
Comments inline.
> @@ -1056,9 +1053,6 @@ unlock_body(void *baton, apr_pool_t *pool)
> int i, max_components = 0, outstanding = 0;
> apr_pool_t *iterpool = svn_pool_create(pool);
>
> - ub->infos = apr_array_make(ub->result_pool, ub->targets->nelts,
> - sizeof(struct unlock_info_t));
> -
> SVN_ERR(ub->fs->vtable->youngest_rev(&youngest, ub->fs, pool));
> SVN_ERR(ub->fs->vtable->revision_root(&root, ub->fs, youngest, pool));
The unlock_body() function has multiple calling sites — svn_fs_fs__unlock()
and unlock_single(). This patch moves the ub->infos initialization into
svn_fs_fs__unlock(), but leaves unlock_single() unchanged. Hence, we will
most likely see another segfault due to us accessing uninitialized memory:
Use of uninitialised value of size 8
at 0x59BA8D6: apr_array_push (...)
by 0x6413D1B: unlock_body (lock.c:1089)
by 0x6414BE6: get_lock (lock.c:1181)
by 0x6412CD1: svn_fs_fs__allow_locked_operation (lock.c:511)
by 0x642233B: commit_body (transaction.c:3251)
by 0x6408E9B: with_lock (fs_fs.c:221)
by 0x6422130: svn_fs_fs__commit (transaction.c:3613)
by 0x6425E0D: svn_fs_fs__commit_txn (tree.c:2224)
by 0x4019B4: lock_expiration (locks-test.c:659)
by 0x4E3DB34: test_thread (svn_test_main.c:525)
by 0x5BE1181: start_thread (pthread_create.c:312)
by 0x5EF230C: clone (clone.S:111)
Invalid write of size 8
at 0x6413D1C: unlock_body (lock.c:1089)
by 0x6414BE6: get_lock (lock.c:1181)
by 0x6412CD1: svn_fs_fs__allow_locked_operation (lock.c:511)
by 0x642233B: commit_body (transaction.c:3251)
by 0x6408E9B: with_lock (fs_fs.c:221)
by 0x6422130: svn_fs_fs__commit (transaction.c:3613)
by 0x6425E0D: svn_fs_fs__commit_txn (tree.c:2224)
by 0x4019B4: lock_expiration (locks-test.c:659)
by 0x4E3DB34: test_thread (svn_test_main.c:525)
by 0x5BE1181: start_thread (pthread_create.c:312)
by 0x5EF230C: clone (clone.S:111)
> + SVN_ERR(create_greek_fs(&fs, &newrev, "obtain-write-lock-failure-test",
> + opts, pool));
> + SVN_ERR(svn_fs_create_access(&access, "bubba", pool));
> + SVN_ERR(svn_fs_set_access(fs, access));
Should probably be named "test-obtain-write-lock-failure".
+ /* Make a read only 'write-lock' file. This prevents any write operations
+ from being executed. */
+ SVN_ERR(svn_io_set_file_read_only("obtain-write-lock-failure-test/write-lock",
+ TRUE, pool));
I suppose there is no reason to use ignore_enoent = TRUE here, right? The
'write-lock' is always there and if it is not, the test shouldn't give a
false positive.
Regards,
Evgeny Kotkov
Received on 2015-02-05 11:50:33 CET