On 10/13/14 3:54 PM, stefan2_at_apache.org wrote:
> Author: stefan2
> Date: Mon Oct 13 22:54:13 2014
> New Revision: 1631598
>
> URL: http://svn.apache.org/r1631598
> Log:
> Add FSFS index checksum verification code to 'svnadmin verify'.
>
> We don't verify the index data against the checksums on every
> access as there is plenty of cross-verification within the indexes,
> between them as well as between index and rev data. Only if some
> inconsistency has been detected and the user wants to trace down
> the source (using 'svnadmin verify'), will we verify that the index
> data has not been tampered with.
>
> * subversion/libsvn_fs_fs/verify.c
> (verify_index_checksum,
> verify_index_checksums): New per rev / pack file verification code.
> (verify_index_consistency): Call the new index checksum test before
> using any of the index contents.
>
> * subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c
> (fuzzing_1_byte_1_rev): Remove the code that would accept certain
> index modifications. Accept only case changes
> in index MD5 digests because they don't affect
> the validity.
[snip]
> @@ -144,14 +120,30 @@ fuzzing_1_byte_1_rev(const char *repo_na
> err = svn_repos_verify_fs3(repos, revision, revision, TRUE, FALSE, FALSE,
> NULL, NULL, NULL, NULL, iterpool);
>
> - /* Benign changes may or may not be detected. */
> - if (!is_legal)
> + /* Case-only changes in checksum digests are not an error.
> + * We allow upper case chars to be used in MD5 checksums in all other
> + * places, thus restricting them here would be inconsistent. */
> + if ( i >= filesize - footer_len /* Within footer */
> + && c_old >= 'a' && c_old <= 'f' /* 'a' to 'f', only appear
> + in checksum digests */
> + && c_new == c_old - 'a' + 'A') /* respective upper case */
> + {
> + if (err)
> + {
> + /* Let us know where we were too strict ... */
> + printf("Detected case change in checksum digest at offset 0x%"
> + APR_UINT64_T_HEX_FMT " (%" APR_OFF_T_FMT ") in r%ld: "
> + "%c -> %c\n", i, i, revision, c_old, c_new);
> +
> + SVN_ERR(err);
> + }
> + }
> + else if (!err)
> {
> /* Let us know where we miss changes ... */
> - if (!err)
> - printf("Undetected mod at offset %"APR_UINT64_T_HEX_FMT
> - " (%"APR_OFF_T_FMT") in r%ld: 0x%02x -> 0x%02x\n",
> - i, i, revision, c_old, c_new);
> + printf("Undetected mod at offset 0x%"APR_UINT64_T_HEX_FMT
> + " (%"APR_OFF_T_FMT") in r%ld: 0x%02x -> 0x%02x\n",
> + i, i, revision, c_old, c_new);
>
> SVN_TEST_ASSERT(err);
> }
>
>
Those printf formats aren't valid on all platforms/builds of APR. In this case
i is an apr_off_t and you're using APR_UINT64_T_HEX_FMT with it. Unfortunately
you can't guarantee that apr_off_t is a 64-bit int. If LFS support isn't
enabled and the platform has its own off_t then APR will use whatever the size
is for that, or just an int if off_t isn't defined. See aprenv.py in APR's
source code for the various scenarios.
Not sure what you want to do about this since it makes the hex output more
difficult to do. I'd probably just remove the hex output since it's a test and
we can probably deal with that bit of convenience.
Received on 2015-01-13 19:59:51 CET