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

Re: BDB segv with NULL checksum

From: Ben Reser <ben_at_reser.org>
Date: Wed, 3 Apr 2013 16:31:04 -0700

You should apply the later one. The check for the kind is redundant.

Just before this check we call svn_fs_base__dag_file_checksum() and
specify that we want tb->base_checksum->kind. Ultimately, this calls
svn_fs_base__rep_contents_checksums() which just does a
svn_checksum_dup on the checksum in the representation of the kind we
asked for. So ultimately, that kind test in the if statement ends up
testing that our functions are following their contract.

On Wed, Apr 3, 2013 at 2:04 PM, Philip Martin
<philip.martin_at_wandisco.com> wrote:
> As part of http://subversion.tigris.org/issues/show_bug.cgi?id=4344 I
> was playing with a BDB repository created by driving the svn_fs API
> directly in repos-test.c:node_locations2. This creates a revision where
> a file has no checksum. A subsequent commit that modifies the file can
> SEGV:
>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7ff8d49ab700 (LWP 4957)]
> 0x00007ff8d6430320 in txn_body_apply_textdelta (baton=0x7ff8da368dc0,
> trail=0x7ff8da368e60) at ../src/subversion/libsvn_fs_base/tree.c:3733
> 3733 if (tb->base_checksum->kind == checksum->kind
>
> because svn_fs_base__dag_file_checksum returns a NULL checksum as the
> documentation allows (repos-test itself doesn't SEGV because it
> doesn't pass a base checksum, but if you interrupt the test after r2 and
> then checkout/commit using a standard client the SEGV occurs).
>
> I can fix the SEGV using this patch:
>
> Index: ../src/subversion/libsvn_fs_base/tree.c
> ===================================================================
> --- ../src/subversion/libsvn_fs_base/tree.c (revision 1464080)
> +++ ../src/subversion/libsvn_fs_base/tree.c (working copy)
> @@ -3730,7 +3730,7 @@
> we're calculating both SHA1 and MD5 checksums somewhere in
> reps-strings.c. Could we keep them both around somehow so this
> check could be more comprehensive? */
> - if (tb->base_checksum->kind == checksum->kind
> + if (checksum && tb->base_checksum->kind == checksum->kind
> && !svn_checksum_match(tb->base_checksum, checksum))
> return svn_checksum_mismatch_err(tb->base_checksum, checksum,
> trail->pool,
>
> If I look at the FSFS code it doesn't do the kind comparison, it was
> removed in r874326 but that's a revert which may have reverted more than
> intended. The FSFS code does:
>
> if (!svn_checksum_match(tb->base_checksum, checksum))
> return svn_checksum_mismatch_err(tb->base_checksum, checksum, pool,
> _("Base checksum mismatch on '%s'"),
> tb->path);
>
> so I can also fix the SEGV with:
>
> Index: ../src/subversion/libsvn_fs_base/tree.c
> ===================================================================
> --- ../src/subversion/libsvn_fs_base/tree.c (revision 1464080)
> +++ ../src/subversion/libsvn_fs_base/tree.c (working copy)
> @@ -3730,8 +3730,7 @@
> we're calculating both SHA1 and MD5 checksums somewhere in
> reps-strings.c. Could we keep them both around somehow so this
> check could be more comprehensive? */
> - if (tb->base_checksum->kind == checksum->kind
> - && !svn_checksum_match(tb->base_checksum, checksum))
> + if (!svn_checksum_match(tb->base_checksum, checksum))
> return svn_checksum_mismatch_err(tb->base_checksum, checksum,
> trail->pool,
> _("Base checksum mismatch on '%s'"),
>
> Which patch should I apply? Should BDB and FSFS be the same?
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
Received on 2013-04-04 01:31:41 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.