On Mon, 2008-07-28 at 16:33 +0200, Stefan Küng wrote:
> The following conditions will trigger a NULL-pointer access:
> * svn library 1.5.1 (I guess 1.5.0 too)
> * merge results in a conflict
> * user provides his own 'resolved' file in the conflict resolver callback
> * the users own 'resolved' file has a path *not* in the same folder than
> the conflicted file is located (best use a completely different path)
>
> Since TortoiseSVN always creates the 'resolved' file in the %TEMP%
> folder, every attempt to resolve a merge conflict in the callback leads
> to a segfault.
>
> The reason for this:
>
> subversion\libsvn_wc\log.c : svn_wc__loggy_copy()
> calls the function loggy_path() with those paths. But in loggy_path(),
> if the path to the resolved file is not a child of the folder where the
> conflict happened, it returns a NULL pointer as the string.
Thanks for the details.
> It seems that loggy_path() is a dangerous function: it assumes that
> paths are relative to each other. For the 'resolved' file path, this
> already leads to a segfault. But I assume that this causes other
> problems with TSVN which I haven't been able yet to determine the exact
> reason. Remember: UI applications don't have the 'current working
> directory' set to what the command line client has it set. In TSVN, I
> had to set the CWD to the %TEMP% folder because some svn functions write
> temp files to the CWD - and if I don't set the CWD it can be anything,
> even the SYSTEM32 folder which is write protected.
According to subversion/libsvn_wc/README, a WC action log can refer only
to files under the working directory that it's operating on. Therefore
the "loggy_path()" function is never expected to return NULL. Files
coming in from outside should have first been copied into the admin temp
directory by the higher level function responsible.
(We could sanity-check the results of loggy_path with SVN_ERR_ASSERT to
enable more graceful failures from this sort of bug. Would you like to
make a patch to do that? There's a couple of ways it could be done.)
In this case, it looks like some part of the huge
svn_wc__merge_internal() is responsible for providing the temporary
file. There are two parts that look likely:
> SVN_ERR(conflict_func(&result, &cdesc, conflict_baton, pool));
> [...]
>
> if (result->save_merged)
> {
> const char *edited_copy;
> /* ### Should use preserved-conflict-file-exts. */
> SVN_ERR(svn_io_open_unique_file2(NULL,
> &edited_copy,
> merge_target,
> ".edited",
> svn_io_file_del_none,
> pool));
> SVN_ERR(svn_wc__loggy_copy
> (log_accum, NULL, adm_access,
> svn_wc__copy_translate,
> /* Look for callback's own merged-file first: */
> result->merged_file
> ? result->merged_file : result_target,
> edited_copy,
> FALSE, pool));
> }
>
> switch (result->choice)
> {
> [...]
> /* For the case of 3-way file merging, we don't
> really distinguish between these return values;
> if the callback claims to have "generally
> resolved" the situation, we still interpret
> that as "OK, we'll assume the merged version is
> good to use". */
> case svn_wc_conflict_choose_merged:
> {
> SVN_ERR(svn_wc__loggy_copy
> (log_accum, NULL, adm_access,
> svn_wc__copy_translate,
> /* Look for callback's own merged-file first: */
> result->merged_file ?
> result->merged_file : result_target,
> merge_target,
> FALSE, pool));
> *merge_outcome = svn_wc_merge_merged;
> contains_conflicts = FALSE;
> goto merge_complete;
> }
I'm guessing one or both of those should be taking a copy of the file by
normal (non-loggy) means, either instead of or before using a loggy
copy.
I can't really take this further at the moment. I hope this is useful.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-29 17:12:01 CEST