You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2015/09/09 20:00:08 UTC

Re: svn commit: r1658848 - in /subversion/trunk/subversion: libsvn_wc/token-map.h libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db_private.h libsvn_wc/wc_db_update_move.c tests/cmdline/move_tests.py tests/libsvn_wc/op-depth-test.c

On 11 February 2015 at 03:42,  <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Wed Feb 11 00:42:01 2015
> New Revision: 1658848
>
> URL: http://svn.apache.org/r1658848
> Log:
> Make the (non recursive) revert db operation properly report tree conflicts
> that it creates by both fixing what is stored in the tree conflict and by
> properly creating notifications.
>
> Note that a recursive revert wouldn't encounter this problem as it
> would just break the moves.
>
Hi Bert,

It seems this commit introduces potential use of uninitialized
variable CONFLICT in subversion/libsvn_wc/wc_db.c:op_revert_txn(). See
below.

> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1658848&r1=1658847&r2=1658848&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Feb 11 00:42:01 2015
> @@ -6738,6 +6738,8 @@ op_revert_txn(void *baton,
>    svn_boolean_t moved_here;
>    int affected_rows;
>    const char *moved_to;
> +  int op_depth_increased = 0;
> +  svn_skel_t *conflict;
CONFLICT local variable will be uninitialized if MOVED_TO is non-zero.

>
>    /* ### Similar structure to op_revert_recursive_txn, should they be
>           combined? */
> @@ -6794,45 +6796,12 @@ op_revert_txn(void *baton,
>      }
>    else
>      {
> -      svn_skel_t *conflict;
> -
>        SVN_ERR(svn_wc__db_read_conflict_internal(&conflict, wcroot,
>                                                  local_relpath,
>                                                  scratch_pool, scratch_pool));
> -      if (conflict)
> -        {
> -          svn_wc_operation_t operation;
> -          svn_boolean_t tree_conflicted;
> -
> -          SVN_ERR(svn_wc__conflict_read_info(&operation, NULL, NULL, NULL,
> -                                             &tree_conflicted,
> -                                             db, wcroot->abspath,
> -                                             conflict,
> -                                             scratch_pool, scratch_pool));
> -          if (tree_conflicted
> -              && (operation == svn_wc_operation_update
> -                  || operation == svn_wc_operation_switch))
> -            {
> -              svn_wc_conflict_reason_t reason;
> -              svn_wc_conflict_action_t action;
> -
> -              SVN_ERR(svn_wc__conflict_read_tree_conflict(&reason, &action,
> -                                                          NULL,
> -                                                          db, wcroot->abspath,
> -                                                          conflict,
> -                                                          scratch_pool,
> -                                                          scratch_pool));
> -
> -              if (reason == svn_wc_conflict_reason_deleted)
> -                SVN_ERR(svn_wc__db_resolve_delete_raise_moved_away(
> -                          db, svn_dirent_join(wcroot->abspath, local_relpath,
> -                                              scratch_pool),
> -                          NULL, NULL /* ### How do we notify this? */,
> -                          scratch_pool));
> -            }
> -        }
>      }
>
> +
>    if (op_depth > 0 && op_depth == relpath_depth(local_relpath))
>      {
>        /* Can't do non-recursive revert if children exist */
> @@ -6857,7 +6826,7 @@ op_revert_txn(void *baton,
>        SVN_ERR(svn_sqlite__bindf(stmt, "isd", wcroot->wc_id,
>                                  local_relpath,
>                                  op_depth));
> -      SVN_ERR(svn_sqlite__step_done(stmt));
> +      SVN_ERR(svn_sqlite__update(&op_depth_increased, stmt));
>
>        SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>                                          STMT_DELETE_WORKING_NODE));
> @@ -6875,6 +6844,54 @@ op_revert_txn(void *baton,
>          SVN_ERR(clear_moved_to(wcroot, local_relpath, scratch_pool));
>      }
>
> +  if (op_depth_increased && conflict)
> +    {
> +      svn_wc_operation_t operation;
> +      svn_boolean_t tree_conflicted;
> +      const apr_array_header_t *locations;
> +
> +      SVN_ERR(svn_wc__conflict_read_info(&operation, &locations, NULL, NULL,
> +                                          &tree_conflicted,
> +                                          db, wcroot->abspath,
> +                                          conflict,
> +                                          scratch_pool, scratch_pool));
And then used here.

I'm not expert in this area, but may you know how to reproduce and fix
this problem because I see several TortoiseSVN crashes [1] due access
to uninitialized variable CONFLICT. Call stack:
[[[
     libsvn_tsvn.dll!svn_wc__conflict_read_info(svn_wc_operation_t *
operation, const apr_array_header_t * * locations, int *
text_conflicted, int * prop_conflicted, int * tree_conflicted,
svn_wc__db_t * conflict_skel, const char * result_pool, const
svn_skel_t * scratch_pool, apr_pool_t *) Line 700    C
>    libsvn_tsvn.dll!op_revert_txn(void * baton, svn_wc__db_wcroot_t * wcroot, const char * local_relpath, apr_pool_t * scratch_pool) Line 6946    C
     libsvn_tsvn.dll!with_triggers(void * baton, svn_wc__db_wcroot_t *
wcroot, const char * local_relpath, apr_pool_t * scratch_pool) Line
3081    C
     libsvn_tsvn.dll!svn_wc__db_op_revert(svn_wc__db_t * db, const
char * local_abspath, svn_depth_t depth, int clear_changelists,
apr_pool_t * scratch_pool, apr_pool_t *) Line 7209    C
     libsvn_tsvn.dll!revert(svn_wc__db_t * db, const char *
local_abspath, svn_depth_t depth, int use_commit_times, int
clear_changelists, int metadata_only, svn_error_t * (void *) *
cancel_func, void * cancel_baton, void (void *, const svn_wc_notify_t
*, apr_pool_t *) * notify_func, void * notify_baton, apr_pool_t *
scratch_pool) Line 743    C
     libsvn_tsvn.dll!svn_wc_revert5(svn_wc_context_t * wc_ctx, const
char * local_abspath, svn_depth_t depth, int use_commit_times, const
apr_array_header_t * changelist_filter, int clear_changelists, int
metadata_only, svn_error_t * (void *) * cancel_func, void *
cancel_baton, void (void *, const svn_wc_notify_t *, apr_pool_t *) *
notify_func, void * notify_baton, apr_pool_t * scratch_pool) Line 995
  C
]]

[1] https://drdump.com/Bug.aspx?ProblemID=146007 (private)

-- 
Ivan Zhakov