On 13.01.2015 19:59, Ben Reser wrote:
> 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.
Since it is a test, what's wrong with just casting the first vararg to
(apr_uint64_t) instead, since we "know" (i.e., hope) that off_t won't
overflow 64 bits ...
-- Brane
Received on 2015-01-13 20:14:07 CET