Diff suffix scanning invalid read

Philip Martin <philip.martin_at_wandisco.com> writes:

> $ valgrind -q .libs/lt-diff-diff3-test 15
> ==6097== Invalid read of size 1
> ==6097== at 0x503FD83: find_identical_suffix (diff_file.c:586)
> ==6097== by 0x5040C45: datasources_open (diff_file.c:815)
> ==6097== by 0x503D6B2: svn_diff_diff3_2 (diff3.c:276)
> ==6097== by 0x5041D3A: svn_diff_file_diff3_2 (diff_file.c:1327)
> ==6097== by 0x401F2F: three_way_merge (diff-diff3-test.c:191)
> ==6097== by 0x4027B7: two_way_diff (diff-diff3-test.c:311)
> ==6097== by 0x405DF8: test_token_compare (diff-diff3-test.c:2589)
> ==6097== by 0x4E34C6A: do_test_num (svn_test_main.c:268)
> ==6097== by 0x4E35686: main (svn_test_main.c:551)
> ==6097== Address 0x138585af is 1 bytes before a block of size 131,072 alloc'd
> ==6097== at 0x4C28BED: malloc (vg_replace_malloc.c:263)
> ==6097== by 0x572DDDB: pool_alloc (apr_pools.c:1463)
> ==6097== by 0x572DF57: apr_palloc_debug (apr_pools.c:1504)
> ==6097== by 0x503FACA: find_identical_suffix (diff_file.c:558)
> ==6097== by 0x5040C45: datasources_open (diff_file.c:815)
> ==6097== by 0x503D6B2: svn_diff_diff3_2 (diff3.c:276)
> ==6097== by 0x5041D3A: svn_diff_file_diff3_2 (diff_file.c:1327)
> ==6097== by 0x401F2F: three_way_merge (diff-diff3-test.c:191)
> ==6097== by 0x4027B7: two_way_diff (diff-diff3-test.c:311)
> ==6097== by 0x405DF8: test_token_compare (diff-diff3-test.c:2589)
> ==6097== by 0x4E34C6A: do_test_num (svn_test_main.c:268)
> ==6097== by 0x4E35686: main (svn_test_main.c:551)

On entry to find_identical_suffix file[0] is

(gdb) p file[0]
$1 = {path = 0x407ee6 "token-compare-original1", file = 0x127cea0,
  size = 131072, chunk = 0, buffer = 0x127cf50 '\n' <repeats 200 times>...,
  curp = 0x129cf48 " @@@\n\300\312&\001", endp = 0x129cf50 "\300\312&\001",
  normalize_state = svn_diff__normalize_state_normal, suffix_start_chunk = -1,
  suffix_offset_in_chunk = 0}

The file data ends at the end of the chunk. The code does

      if (file_for_suffix[i].chunk == file[i].chunk)
        {
          /* Prefix ended in last chunk, so we can reuse the prefix buffer */
          file_for_suffix[i].buffer = file[i].buffer;
        }
      else
        {
          /* There is at least more than 1 chunk,
             so allocate full chunk size buffer */
          file_for_suffix[i].buffer = apr_palloc(pool, CHUNK_SIZE);
          SVN_ERR(read_chunk(file_for_suffix[i].file, file_for_suffix[i].path,
                             file_for_suffix[i].buffer, length[i],
                             chunk_to_offset(file_for_suffix[i].chunk),
                             pool));
        }
      file_for_suffix[i].endp = file_for_suffix[i].buffer + length[i];
      file_for_suffix[i].curp = file_for_suffix[i].endp - 1;

and allocates a new chunk so that file_for_suffix[0] is

(gdb) p file_for_suffix[0]
$3 = {path = 0x407ee6 "token-compare-original1", file = 0x127cea0,
  size = 131072, chunk = 1, buffer = 0x12bd0d0 'A' <repeats 200 times>...,
  curp = 0x12bd0cf "", endp = 0x12bd0d0 'A' <repeats 200 times>...,
  normalize_state = svn_diff__normalize_state_normal, suffix_start_chunk = 0,
  suffix_offset_in_chunk = 0}

file_for_suffix[0=.endp is 1 byte before file_for_suffix[0].buffer and
so dereferencing it is not valid.

How are the chunks supposed to work? When the file data ends at the end
of a chunk is there supposed to be another valid chunk with no data?

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download