[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] svn_fs_fs__get_changes: do not crash without cache

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Tue, 10 Dec 2013 07:34:52 +0400

Hi,

I've noticed that the svn_fs_fs__get_changes function currently invokes
undefined behavior when there is no CHANGES_CACHE present. According to the
fs_fs_data_t contract, this is a perfectly valid situation (see
subversion\libsvn_fs_fs\fs.h around trunk_at_1549686):
[[[
  /* Private (non-shared) FSFS-specific data for each svn_fs_t object.
     Any caches in here may be NULL. */
]]]

However, explicitly setting the CHANGES_CACHE to NULL leads to a massive amount
of failing tests (1085 fails on my 64-bit Ubuntu machine, some of them are
segfaults). This erroneous behavior was introduced in r1512904 [1] and happens
due to the usage of the uninitialized variable FOUND when the CHANGES_CACHE
is absent. The behavior in this case is undefined and that will likely lead to
a crash. Here is a tip from valgrind:
[[[
  Conditional jump or move depends on uninitialised value(s)
    at 0x4852AF: svn_fs_fs__get_changes (cached_data.c:2183)
    by 0x469E4C: svn_fs_fs__paths_changed (transaction.c:889)
    by 0x475CDE: fs_paths_changed (tree.c:3291)
    by 0x44DCE7: svn_fs_paths_changed2 (fs-loader.c:1006)
    ...

  Use of uninitialised value of size 8
    at 0x469E8C: svn_fs_fs__paths_changed (transaction.c:892)
    by 0x475CDE: fs_paths_changed (tree.c:3291)
    by 0x44DCE7: svn_fs_paths_changed2 (fs-loader.c:1006)
    ...
]]]

This happens with both FSFS and FSX, so I've attached two patches to fix this
issue (these patches fix the svn_fs_fs__get_changes and svn_fs_x__get_changes
functions accordingly).

[1] http://svn.apache.org/viewvc?view=revision&revision=r1512904

Thanks and regards,
Evgeny Kotkov

Received on 2013-12-10 04:38:03 CET

This is an archived mail posted to the Subversion Dev mailing list.