Re: [PATCH 00/11] rebase: reset_head() related fixes and improvements

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Oct 2, 2021, 12:27 UTC
Message-ID: <ddf5e9c2-3211-cec2-cd18-2a083e279ccb@gmail.com>
In-Reply-To: <xmqq4ka02fc4.fsf@gitster.g>

Hi Junio

On 02/10/2021 05:58, Junio C Hamano wrote:

Show 5 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> When merged with other topics in flight in 'seen', this seems to
>> fail the t1092 test (most likely, ds/add-rm-with-sparse-index is
>> what this interacts badly with).

Oh dear, thanks for letting me know.

Show 18 quoted lines

> There are Two CI runs at GitHub:
> 
>   - https://github.com/git/git/actions/runs/1297117791 (d3a1c4e)
>   - https://github.com/git/git/actions/runs/1297232973 (a1108c2)
> 
> The difference between the former (which passes all the tests) and
> he latter (which fails) are only two topics:
> 
>      $ git log --first-parent --oneline d3a1c4e..a1108c2
>      a1108c2b1b Merge branch 'hn/reftable' into seen
>      e575f29006 Merge branch 'pw/fix-some-issues-in-reset-head' into seen
> 
> I think the following is the same failure I saw earlier
> 
>     https://github.com/git/git/runs/3773780195?check_suite_focus=true#step:6:5033
> 
> that the write-tree codepath hits assertion failure by detecting a
> corruption in the cache-tree data structure.

The test that fails (t1092-sparse-checkout-compatibility.sh:'merge, cherry-pick, and rebase') was introduced by c0b99303db ("t1092: add cherry-pick, rebase tests", 2021-09-08) and is in v2.33.0. It does not test the "apply" backend of rebase, changing it to do so makes it fail on the current master as that backend already uses reset_head() for the initial checkout so it is an existing bug that is exposed by the changes in this series. It seems to be a use-after-free (see below) I'll try and investigate but I've got no idea what is going on at the moment - the index is not my area of expertise.

Best Wishes

Phillip

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 
0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
     #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
     #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #4 0x557cbe830a2b in cache_tree_verify 
/home/phil/src/git/cache-tree.c:910
     #5 0x557cbea53741 in write_locked_index 
/home/phil/src/git/read-cache.c:3250
     #6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
     #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
     #8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
     #9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
     #10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
     #11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
     #12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
     #13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
     #14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region 
[0x606000001b20,0x606000001b58)
freed by thread T0 here:
     #0 0x7fdd4bacff19 in __interceptor_free 
/build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
     #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
     #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
     #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
     #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
     #5 0x557cbeb2557a in ensure_full_index 
/home/phil/src/git/sparse-index.c:310
     #6 0x557cbea45c4a in index_name_stage_pos 
/home/phil/src/git/read-cache.c:588
     #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
     #8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #11 0x557cbe830a2b in cache_tree_verify 
/home/phil/src/git/cache-tree.c:910
     #12 0x557cbea53741 in write_locked_index 
/home/phil/src/git/read-cache.c:3250
     #13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
     #14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
     #15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
     #16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
     #17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
     #18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
     #19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
     #20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
     #0 0x7fdd4bad0459 in __interceptor_calloc 
/build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
     #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
     #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
     #3 0x557cbe82b7d8 in prime_cache_tree_rec 
/home/phil/src/git/cache-tree.c:763
     #4 0x557cbe82b837 in prime_cache_tree_rec 
/home/phil/src/git/cache-tree.c:764
     #5 0x557cbe82b837 in prime_cache_tree_rec 
/home/phil/src/git/cache-tree.c:764
     #6 0x557cbe8304e1 in prime_cache_tree 
/home/phil/src/git/cache-tree.c:779
     #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
     #8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
     #9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
     #10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
     #11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
     #12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
     #13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
     #14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

SUMMARY: AddressSanitizer: heap-use-after-free 
/home/phil/src/git/cache-tree.c:863 in verify_one
Shadow bytes around the buggy address:
   0x0c0c7fff8310: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
   0x0c0c7fff8320: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fa
   0x0c0c7fff8330: fa fa fa fa 00 00 00 00 00 00 00 02 fa fa fa fa
   0x0c0c7fff8340: fd fd fd fd fd fd fd fa fa fa fa fa 00 00 00 00
   0x0c0c7fff8350: 00 00 00 02 fa fa fa fa fd fd fd fd fd fd fd fa
=>0x0c0c7fff8360: fa fa fa fa[fd]fd fd fd fd fd fd fa fa fa fa fa
   0x0c0c7fff8370: 00 00 00 00 00 00 00 02 fa fa fa fa fd fd fd fd
   0x0c0c7fff8380: fd fd fd fa fa fa fa fa 00 00 00 00 00 00 00 02
   0x0c0c7fff8390: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
   0x0c0c7fff83a0: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
   0x0c0c7fff83b0: fd fd fd fa fa fa fa fa 00 00 00 00 00 00 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
   Addressable:           00
   Partially addressable: 01 02 03 04 05 06 07
   Heap left redzone:       fa
   Freed heap region:       fd
   Stack left redzone:      f1
   Stack mid redzone:       f2
   Stack right redzone:     f3
   Stack after return:      f5
   Stack use after scope:   f8
   Global redzone:          f9
   Global init order:       f6
   Poisoned by user:        f7
   Container overflow:      fc
   Array cookie:            ac
   Intra object redzone:    bb
   ASan internal:           fe
   Left alloca redzone:     ca
   Right alloca redzone:    cb
   Shadow gap:              cc
==74345==ABORTING

Show 11 quoted lines

> e575f29006 (i.e. without the reftable topic) fails t1092.  If you
> revert e575f29006^2 (i.e. the "do not fork 'git checkout'") from
> that merge, all tests pass including t1092.
> 
> The reftable topic is queued near the tip of 'seen' not necessarily
> because it _breaks_ CI (I do not think it does), but it needed a
> handful of fixup commits on top.  The topic needs rerolling with the
> fixes squashed in.
> 
> Thanks.
>