Skip to content

Sync with 'maint' - #6

Open
Faisal M. AlOtaibi (hub966) wants to merge 1540 commits into
seenfrom
master
Open

Faisal M. AlOtaibi (hub966) wants to merge 1540 commits into
seenfrom
master

Conversation

@hub966

@hub966 hub966 commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Thanks for taking the time to contribute to Git! Please be advised that the
Git community does not use github.com for their contributions. Instead, we use
a mailing list (git@vger.kernel.org) for code submissions, code reviews, and
bug reports. Nevertheless, you can use GitGitGadget (https://gitgitgadget.github.io/)
to conveniently send your Pull Requests commits to our mailing list.

For a single-commit pull request, please leave the pull request description
empty
: your commit message itself should describe your changes.

Please read the "guidelines for contributing" linked above!

Faisal M. AlOtaibi (hub966) pushed a commit that referenced this pull request Jul 17, 2026
In prepare_bases() we do a custom revision walk, separate from the main
format-patch walk. After we finish, we fail to call release_revisions(),
possibly leaking its contents.

We failed to notice it so far because the revision machinery doesn't
always allocate. But at least one case can trigger the leak: if a commit
graph is present, then the topo-walk allocates revs.topo_walk_info and
some associated data structures. You can see it in the test suite by
running:

  make SANITIZE=leak
  cd t
  GIT_TEST_COMMIT_GRAPH=1 ./t4014-format-patch.sh

which yields many entries like:

  ==git==3687620==ERROR: LeakSanitizer: detected memory leaks
  Direct leak of 200 byte(s) in 1 object(s) allocated from:
      #0 0x7f4ccba185cb in malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:74
      #1 0x55cd452cdd0b in do_xmalloc wrapper.c:55
      #2 0x55cd452cdd9d in xmalloc wrapper.c:76
      #3 0x55cd45255473 in init_topo_walk revision.c:3845
      #4 0x55cd45255bef in prepare_revision_walk revision.c:4017
      #5 0x55cd44ffec40 in prepare_bases builtin/log.c:1872
      #6 0x55cd450010ec in cmd_format_patch builtin/log.c:2439

The un-released rev_info has been there since the code was added in
fa2ab86 (format-patch: add '--base' option to record base tree info,
2016-04-26), but back then we didn't even have a way to release rev_info
resources! The actual leak probably started around f0d9cc4
(revision.c: begin refactoring --topo-order logic, 2018-11-01), but it's
hard to bisect because there were so many other unrelated leaks back
then.

So I'm not sure exactly when the leak started beyond "long ago", but it
is easy-ish to find now (since we've plugged all those other leaks) and
the solution is clear.

I didn't add a new test since we can demonstrate it with the existing
ones, but it does require tweaking a test variable. We might consider
ways to get more automatic leak-checking coverage there, but I think it
should be done outside of this fix.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Copilot AI review requested due to automatic review settings July 24, 2026 19:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Faisal M. AlOtaibi (hub966) pushed a commit that referenced this pull request Jul 25, 2026
The "log" reftable block stores reflog information. This information is
compressed using zlib. The inflated size is stored in the block header
so that callers can easily learn ahead of time how large of a buffer
they have to allocate to inflate the data in a single pass. So to
reconstruct the full inflated block we:

  - Copy over the header as-is, as it's not deflated.

  - Append the inflated data to the buffer.

The inflated block size stored in the header also includes the length of
the header itself. So to figure out the bytes that should be inflated by
zlib we need to subtract the header size, which is trusted data, from
the block size, which is untrusted data derived from the block header.

While we do verify that we were able to inflate all data as expected, we
don't verify ahead of time that the encoded block length is larger than
the header length. This can lead to an underflow, which makes zlib
assume that it can write more data into the target buffer than we have
allocated. The result is an out-of-bounds write:

  ==1422297==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c1ff6de5231 at pc 0x55555579a628 bp 0x7fffffff4f10 sp 0x7fffffff46d0
  WRITE of size 4 at 0x7c1ff6de5231 thread T0
      #0 0x55555579a627 in __asan_memcpy (./build/t/unit-tests+0x246627)
      #1 0x55555598b093 in reftable_block_init ./build/../reftable/block.c:277:3
      #2 0x555555813701 in test_reftable_block__corrupt_log_block_size ./build/../t/unit-tests/u-reftable-block.c:495:20
      #3 0x5555557f684e in clar_run_test ./build/../t/unit-tests/clar/clar.c:335:3
      #4 0x5555557f2e69 in clar_run_suite ./build/../t/unit-tests/clar/clar.c:431:3
      #5 0x5555557f2882 in clar_test_run ./build/../t/unit-tests/clar/clar.c:636:4
      #6 0x5555557f375f in clar_test ./build/../t/unit-tests/clar/clar.c:687:11
      #7 0x5555557fa49d in cmd_main ./build/../t/unit-tests/unit-test.c:62:8
      #8 0x55555584af4a in main ./build/../common-main.c:9:11
      #9 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b284) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #10 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b337) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #11 0x555555694c24 in _start (./build/t/unit-tests+0x140c24)

  0x7c1ff6de5231 is located 0 bytes after 1-byte region [0x7c1ff6de5230,0x7c1ff6de5231)
  allocated by thread T0 here:
      #0 0x55555579db1b in realloc.part.0 asan_malloc_linux.cpp.o
      #1 0x5555559868d7 in reftable_realloc ./build/../reftable/basics.c:36:9
      #2 0x55555598a98f in reftable_alloc_grow ./build/../reftable/basics.h:229:10
      #3 0x55555598ae58 in reftable_block_init ./build/../reftable/block.c:269:3
      #4 0x555555813701 in test_reftable_block__corrupt_log_block_size ./build/../t/unit-tests/u-reftable-block.c:495:20
      #5 0x5555557f684e in clar_run_test ./build/../t/unit-tests/clar/clar.c:335:3
      #6 0x5555557f2e69 in clar_run_suite ./build/../t/unit-tests/clar/clar.c:431:3
      #7 0x5555557f2882 in clar_test_run ./build/../t/unit-tests/clar/clar.c:636:4
      #8 0x5555557f375f in clar_test ./build/../t/unit-tests/clar/clar.c:687:11
      #9 0x5555557fa49d in cmd_main ./build/../t/unit-tests/unit-test.c:62:8
      #10 0x55555584af4a in main ./build/../common-main.c:9:11
      #11 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b284) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #12 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b337) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #13 0x555555694c24 in _start (./build/t/unit-tests+0x140c24)

  SUMMARY: AddressSanitizer: heap-buffer-overflow (./build/t/unit-tests+0x246627) in __asan_memcpy
  Shadow bytes around the buggy address:
    0x7c1ff6de4f80: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
    0x7c1ff6de5000: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
    0x7c1ff6de5080: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
    0x7c1ff6de5100: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
    0x7c1ff6de5180: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fd
  =>0x7c1ff6de5200: fa fa 04 fa fa fa[01]fa fa fa fa fa fa fa fa fa
    0x7c1ff6de5280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c1ff6de5300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c1ff6de5380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c1ff6de5400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c1ff6de5480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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

Fix the bug by adding a sanity check and add a unit test.

Reported-by: oxsignal <awo@kakao.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Faisal M. AlOtaibi (hub966) pushed a commit that referenced this pull request Jul 25, 2026
The block size is read from the block header, which is untrusted data.
We use it without verification to access the restart count at the end of
the block as well as to compute the restart table offset. With a bogus
block size that exceeds the data we have actually read this can lead to
an out-of-bounds read:

  ==2274138==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c3ff6de2e3f at pc 0x55555598c6ea bp 0x7fffffff4ee0 sp 0x7fffffff4ed8
  READ of size 1 at 0x7c3ff6de2e3f thread T0
      #0 0x55555598c6e9 in reftable_get_be16 /home/pks/Development/git/build/../reftable/basics.h:119:20
      #1 0x55555598c252 in reftable_block_init /home/pks/Development/git/build/../reftable/block.c:343:18
      #2 0x555555813c70 in test_reftable_block__corrupt_block_size /home/pks/Development/git/build/../t/unit-tests/u-reftable-block.c:531:20
      #3 0x5555557f684e in clar_run_test /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:335:3
      #4 0x5555557f2e69 in clar_run_suite /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:431:3
      #5 0x5555557f2882 in clar_test_run /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:636:4
      #6 0x5555557f375f in clar_test /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:687:11
      #7 0x5555557fa49d in cmd_main /home/pks/Development/git/build/../t/unit-tests/unit-test.c:62:8
      #8 0x55555584b8aa in main /home/pks/Development/git/build/../common-main.c:9:11
      #9 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/8kvxvr3pmsypxiypq4g8zy13glnfr7nx-glibc-2.42-67/lib/libc.so.6+0x2b284) (BuildId: 5a702452a01df1d7d50ce0663acec7be3c71fd4d)
      #10 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/8kvxvr3pmsypxiypq4g8zy13glnfr7nx-glibc-2.42-67/lib/libc.so.6+0x2b337) (BuildId: 5a702452a01df1d7d50ce0663acec7be3c71fd4d)
      #11 0x555555694c24 in _start (/home/pks/Development/git/build/t/unit-tests+0x140c24)

  0x7c3ff6de2e3f is located 0 bytes after 47-byte region [0x7c3ff6de2e10,0x7c3ff6de2e3f)
  allocated by thread T0 here:
      #0 0x55555579e95b in malloc (/home/pks/Development/git/build/t/unit-tests+0x24a95b)
      #1 0x5555559871c2 in reftable_malloc /home/pks/Development/git/build/../reftable/basics.c:24:9
      #2 0x5555559872e8 in reftable_calloc /home/pks/Development/git/build/../reftable/basics.c:54:6
      #3 0x55555598f0d3 in reftable_buf_read_data /home/pks/Development/git/build/../reftable/blocksource.c:67:2
      #4 0x55555598ea7e in block_source_read_data /home/pks/Development/git/build/../reftable/blocksource.c:41:19
      #5 0x55555598c555 in read_block /home/pks/Development/git/build/../reftable/block.c:224:9
      #6 0x55555598b69e in reftable_block_init /home/pks/Development/git/build/../reftable/block.c:258:9
      #7 0x555555813c70 in test_reftable_block__corrupt_block_size /home/pks/Development/git/build/../t/unit-tests/u-reftable-block.c:531:20
      #8 0x5555557f684e in clar_run_test /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:335:3
      #9 0x5555557f2e69 in clar_run_suite /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:431:3
      #10 0x5555557f2882 in clar_test_run /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:636:4
      #11 0x5555557f375f in clar_test /home/pks/Development/git/build/../t/unit-tests/clar/clar.c:687:11
      #12 0x5555557fa49d in cmd_main /home/pks/Development/git/build/../t/unit-tests/unit-test.c:62:8
      #13 0x55555584b8aa in main /home/pks/Development/git/build/../common-main.c:9:11
      #14 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/8kvxvr3pmsypxiypq4g8zy13glnfr7nx-glibc-2.42-67/lib/libc.so.6+0x2b284) (BuildId: 5a702452a01df1d7d50ce0663acec7be3c71fd4d)
      #15 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/8kvxvr3pmsypxiypq4g8zy13glnfr7nx-glibc-2.42-67/lib/libc.so.6+0x2b337) (BuildId: 5a702452a01df1d7d50ce0663acec7be3c71fd4d)
      #16 0x555555694c24 in _start (/home/pks/Development/git/build/t/unit-tests+0x140c24)

  SUMMARY: AddressSanitizer: heap-buffer-overflow /home/pks/Development/git/build/../reftable/basics.h:119:20 in reftable_get_be16
  Shadow bytes around the buggy address:
    0x7c3ff6de2b80: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
    0x7c3ff6de2c00: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
    0x7c3ff6de2c80: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fa
    0x7c3ff6de2d00: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fa
    0x7c3ff6de2d80: fa fa 00 00 00 00 00 00 fa fa fd fd fd fd fd fd
  =>0x7c3ff6de2e00: fa fa 00 00 00 00 00[07]fa fa fa fa fa fa fa fa
    0x7c3ff6de2e80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c3ff6de2f00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c3ff6de2f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c3ff6de3000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    0x7c3ff6de3080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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

Verify that the claimed block size fits into the block data before using
it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Faisal M. AlOtaibi (hub966) pushed a commit that referenced this pull request Jul 25, 2026
The restart count is stored in the last two bytes of a block. We use it
without verification to compute the offset of the restart table. With a
bogus restart count that is large enough this computation underflows,
and the subsequent reads via the restart table access out-of-bounds
memory:

  ==129439==ERROR: AddressSanitizer: SEGV on unknown address 0x7d90f6dcd0ad (pc 0x55555598ce89 bp 0x7fffffff4ed0 sp 0x7fffffff4e80 T0)
  ==129439==The signal is caused by a READ memory access.
      #0 0x55555598ce89 in reftable_get_be24 ./git/build/../reftable/basics.h:125:9
      #1 0x55555598eabf in block_restart_offset ./git/build/../reftable/block.c:407:9
      #2 0x55555598e5d5 in restart_needle_less ./git/build/../reftable/block.c:431:17
      #3 0x5555559887e2 in binsearch ./git/build/../reftable/basics.c:165:13
      #4 0x55555598dfec in block_iter_seek_key ./git/build/../reftable/block.c:529:6
      #5 0x555555814517 in test_reftable_block__corrupt_restart_count ./git/build/../t/unit-tests/u-reftable-block.c:593:15
      #6 0x5555557f684e in clar_run_test ./git/build/../t/unit-tests/clar/clar.c:335:3
      #7 0x5555557f2e69 in clar_run_suite ./git/build/../t/unit-tests/clar/clar.c:431:3
      #8 0x5555557f2882 in clar_test_run ./git/build/../t/unit-tests/clar/clar.c:636:4
      #9 0x5555557f375f in clar_test ./git/build/../t/unit-tests/clar/clar.c:687:11
      #10 0x5555557fa49d in cmd_main ./git/build/../t/unit-tests/unit-test.c:62:8
      #11 0x55555584c12a in main ./git/build/../common-main.c:9:11
      #12 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b284) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #13 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b337) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #14 0x555555694c24 in _start (./git/build/t/unit-tests+0x140c24)

  ==129439==Register values:
  rax = 0x00007d90f6dcd0ad  rbx = 0x00007fffffff4f20  rcx = 0xf2f2f2f8f2f2f2f8  rdx = 0x0000000000000000
  rdi = 0x00007d90f6dcd0ad  rsi = 0x0000000000007fff  rbp = 0x00007fffffff4ed0  rsp = 0x00007fffffff4e80
   r8 = 0x0000000000000000   r9 = 0x0000000000000000  r10 = 0x0000000000000000  r11 = 0x0000000000000017
  r12 = 0x00007fffffff58e8  r13 = 0x0000000000000001  r14 = 0x00007ffff7ffd000  r15 = 0x00005555560550b0
  AddressSanitizer can not provide additional info.
  SUMMARY: AddressSanitizer: SEGV ./git/build/../reftable/basics.h:125:9 in reftable_get_be24

Verify that the restart table actually fits into the block.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Faisal M. AlOtaibi (hub966) pushed a commit that referenced this pull request Jul 25, 2026
Restart points encode records in a given block that do not use prefix
compression and that can thus immediately be seeked to. These offsets
are encoded in the restart table, where each offset needs to point at
one of the records of the block. We do not verify this though, so a
bogus restart offset may cause an out-of-bounds read:

  ==1472280==ERROR: AddressSanitizer: SEGV on unknown address 0x7d8ff7de5f7f (pc 0x55555599502b bp 0x7fffffff4df0 sp 0x7fffffff4d40 T0)
  ==1472280==The signal is caused by a READ memory access.
      #0 0x55555599502b in get_var_int ./git/build/../reftable/record.c:30:6
      #1 0x555555995c2a in reftable_decode_keylen ./git/build/../reftable/record.c:177:6
      #2 0x55555598e85c in restart_needle_less ./git/build/../reftable/block.c:455:6
      #3 0x55555598895f in binsearch ./git/build/../reftable/basics.c:175:9
      #4 0x55555598e189 in block_iter_seek_key ./git/build/../reftable/block.c:543:6
      #5 0x555555814aee in test_reftable_block__corrupt_restart_offset ./git/build/../t/unit-tests/u-reftable-block.c:636:20
      #6 0x5555557f684e in clar_run_test ./git/build/../t/unit-tests/clar/clar.c:335:3
      #7 0x5555557f2e69 in clar_run_suite ./git/build/../t/unit-tests/clar/clar.c:431:3
      #8 0x5555557f2882 in clar_test_run ./git/build/../t/unit-tests/clar/clar.c:636:4
      #9 0x5555557f375f in clar_test ./git/build/../t/unit-tests/clar/clar.c:687:11
      #10 0x5555557fa49d in cmd_main ./git/build/../t/unit-tests/unit-test.c:62:8
      #11 0x55555584c25a in main ./git/build/../common-main.c:9:11
      #12 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b284) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #13 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b337) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #14 0x555555694c24 in _start (./git/build/t/unit-tests+0x140c24)

  ==1472280==Register values:
  rax = 0x00007d8ff7de5f7f  rbx = 0x00007fffffff4e00  rcx = 0x00007d8ff7de5f80  rdx = 0x00007bfff5b6af60
  rdi = 0x00007bfff5b6af40  rsi = 0x00007bfff592dfa0  rbp = 0x00007fffffff4df0  rsp = 0x00007fffffff4d40
   r8 = 0x00000000ff00002b   r9 = 0x00007d8ff7de5f7f  r10 = 0x00000f7ffeb25bf0  r11 = 0xf3f30000f1f1f1f1
  r12 = 0x00007fffffff58f8  r13 = 0x0000000000000001  r14 = 0x00007ffff7ffd000  r15 = 0x0000555556055fd0
  AddressSanitizer can not provide additional info.
  SUMMARY: AddressSanitizer: SEGV ./git/build/../reftable/record.c:30:6 in get_var_int

Guard against such restart offsets and signal an error to the caller via
`args.error`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Faisal M. AlOtaibi (hub966) pushed a commit that referenced this pull request Jul 25, 2026
When seeking an iterator to an arbitrary offset we may return a positive
value in case the offset points beyond the block. This makes sense when
iterating through multiple blocks of the same section, as the positive
value indicates to us that we're at the end of the table.

But when the offset originates from a section or index offset it is
supposed to point at a valid block, so an out-of-bounds value means that
the table is corrupt. Treating it as a normal end-of-iteration causes us
to silently report an empty section instead of surfacing the corruption,
and we are left with a partially-initialized block. This may later on
cause a NULL pointer exception:

  ==1486841==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55555598e02c bp 0x7fffffff4eb0 sp 0x7fffffff4e70 T0)
  ==1486841==The signal is caused by a READ memory access.
  ==1486841==Hint: address points to the zero page.
      #0 0x55555598e02c in reftable_block_type ./git/build/../reftable/block.c:392:9
      #1 0x55555598ee6e in block_iter_seek_key ./git/build/../reftable/block.c:536:35
      #2 0x5555559ae553 in table_iter_seek_linear ./git/build/../reftable/table.c:344:8
      #3 0x5555559adbff in table_iter_seek ./git/build/../reftable/table.c:450:9
      #4 0x5555559ada9c in table_iter_seek_void ./git/build/../reftable/table.c:460:9
      #5 0x555555992872 in reftable_iterator_seek_log_at ./git/build/../reftable/iter.c:281:9
      #6 0x555555992953 in reftable_iterator_seek_log ./git/build/../reftable/iter.c:287:9
      #7 0x55555583aa78 in test_reftable_table__seek_invalid_log_offset ./git/build/../t/unit-tests/u-reftable-table.c:257:20
      #8 0x5555557f684e in clar_run_test ./git/build/../t/unit-tests/clar/clar.c:335:3
      #9 0x5555557f2e69 in clar_run_suite ./git/build/../t/unit-tests/clar/clar.c:431:3
      #10 0x5555557f2882 in clar_test_run ./git/build/../t/unit-tests/clar/clar.c:636:4
      #11 0x5555557f375f in clar_test ./git/build/../t/unit-tests/clar/clar.c:687:11
      #12 0x5555557fa49d in cmd_main ./git/build/../t/unit-tests/unit-test.c:62:8
      #13 0x55555584cffa in main ./git/build/../common-main.c:9:11
      #14 0x7ffff7a2b284 in __libc_start_call_main (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b284) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #15 0x7ffff7a2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/57iz36553175g3178pvxjij8z5rcsd4n-glibc-2.42-61/lib/libc.so.6+0x2b337) (BuildId: 8ae0b698f2d4e727f569f64bb166e08ae30bd077)
      #16 0x555555694c24 in _start (./git/build/t/unit-tests+0x140c24)

  ==1486841==Register values:
  rax = 0x0000000000000000  rbx = 0x00007fffffff4ec0  rcx = 0x0000000000000000  rdx = 0x00007cfff6e2bd58
  rdi = 0x00007cfff6e2bd58  rsi = 0x00007bfff5da1020  rbp = 0x00007fffffff4eb0  rsp = 0x00007fffffff4e70
   r8 = 0x0000000000000000   r9 = 0x0000000000000002  r10 = 0x0000000000000000  r11 = 0x0000000000000017
  r12 = 0x00007fffffff5908  r13 = 0x0000000000000001  r14 = 0x00007ffff7ffd000  r15 = 0x0000555556056e90
  AddressSanitizer can not provide additional info.
  SUMMARY: AddressSanitizer: SEGV ./git/build/../reftable/block.c:392:9 in reftable_block_type
  ==1486841==ABORTING

Fix this by returning a proper error in `table_iter_seek_to()` when the
offset ranges beyond the block.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren (newren) and others added 6 commits August 12, 2026 07:22
revs.pretty_given is only ever read in builtin/log.c, and nothing from
builtin/log.c is ever called from sequencer.c.  So setting this variable
cannot do anything.

This was introduced in commit 62db524 ("rebase -i: generate the
script via rebase--helper", 2017-07-14), which used `git rev-list` even
though its commit message describes the logic as having been based on
`git log`.  Because of this, I am guessing this line was copied or
ported from part of builtin/log.c without recognizing that this line was
not doing anything and could be removed.

It's certainly not doing anything now, though, so remove it.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
…d-alternates

* ps/odb-make-creation-pluggable:
  odb: make creation of on-disk structures pluggable
  odb/source: introduce function to map source type to name
  setup: defer object database creation
  setup: handle ODB-related environment variables in `odb_new()`
  setup: detangle loading of loose object maps
  loose: load loose object map for the correct source
Many of the tests in t7900 operate inside the main trash repository
that's set up by default by our test suite. This is overall quite
fragile as we're exercising repository maintenance in those tests, and
maintenance is of course intricately tied towards the on-disk state of a
repository. Consequently, the tests can easily impact one another.

Furthermore, in the next commit we'll have to modify the environment in
a handful of those tests. As tests don't run in a subshell, doing so
would impact all subsequent tests by default, as well.

Adapt exactly those tests to use a throwaway repository. This makes the
tests more neatly self-contained and allows us to trivially modify the
environment in the next commit.

Note that we adapt calls to `test_config ()` to use git-config(1)
instead. This is because on the one hand we don't need the auto-revert
logic of `test_config ()` as we're using a throwaway repository anyway.
On the other hand it's not possible to use `test_config ()` as it uses
`test_when_finished ()`, which errors out when we run it in a subshell.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of our tests for whether "maintenance.strategy" is being respected
in t7900 is flaky in our CI systems:

    + GIT_TRACE2_EVENT=/tmp/test-output/trash directory.t7900-maintenance/repo/trace2.txt git -c maintenance.strategy=incremental maintenance run --quiet
    + test_maintenance_tasks trace2.txt
    + cat
    + sed -ne s/.*"region_enter".*"category":"maintenance\([^"]*\)".*"label":"\([^"][^"]*\)".*/\2\1/p trace2.txt
    + test_cmp expect actual
    + test 2 -ne 2
    + eval /usr/bin/diff -u "$@"
    + /usr/bin/diff -u expect actual
    --- expect	2026-08-07 06:20:51.388322602 +0000
    +++ actual	2026-08-07 06:20:51.388322602 +0000
    @@ -1,2 +0,0 @@
    -gc foreground
    -gc

When running with the "incremental" strategy, we expect two git-gc(1)
tasks to have been executed, but sometimes the test simply doesn't
execute any of those tasks.

A first hunch may be that maybe the disk-state is sometimes different
and thus we decide not to run maintenance. But git-maintenance(1)
doesn't run with the "--auto" switch, so we should execute those tasks
regardless of the on-disk state.

But there's a second condition that may cause us to not execute tasks,
namely when the "maintenance.lock" file exists due to a concurrently
running git-maintenance(1) process. We usually disable auto-maintenance
from detaching in our test suite to avoid exactly these kinds of race
conditions by exporting `GIT_TEST_MAINT_AUTO_DETACH=false`. But in t7900
we unset "GIT_TEST_MAINT_AUTO_DETACH" and thus enable the auto-detach
logic. The intent of this is to exercise git-maintenance(1) closer to
how it would run in a real-world scenario, but it does cause us to race
when the detached maintenance job that was triggered by `test_commit()`
lives long enough.

We could trivially fix this race by disabling auto-maintenance for this
specific test. But that doesn't fix this class of races in this test
suite: while I haven't seen any of the other tests fail in the same way,
a bunch of them have this race, as well.

Instead, let's retain "GIT_TEST_MAINT_AUTO_DETACH" and only unset it as
required.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_active_slot() duplicates the default curl handle via
curl_easy_duphandle() to create a per-slot session handle. The
return value is stored directly in slot->curl without checking
for NULL. curl_easy_duphandle() can return NULL when memory
allocation fails internally, and the libcurl documentation
explicitly states this possibility.

When this happens, slot->curl is NULL and the very next operation
(curl_easy_setopt on line 1632 for CURLOPT_COOKIEFILE) passes
NULL as the curl handle, which is undefined behavior in libcurl
and typically crashes.

Every HTTP operation in git goes through get_active_slot(), so
this affects all remote-https, remote-http, and HTTP-based
operations (clone, fetch, push over HTTP, bundle-uri downloads).

Add a NULL check and die() with a clear message. There is no
reasonable recovery from a failed handle duplication: the process
is out of memory and cannot perform any HTTP operation.

Pointed out by Coverity.

Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
show_editor() calls launch_editor() to open the user's editor on
the configuration file, but discards the return value and
unconditionally returns 0 (success). When the editor fails to
launch (e.g., $EDITOR is not found, or the editor exits with a
nonzero status), the caller receives no indication that anything
went wrong.

This affects "git config edit" and "git config --edit": the
command silently succeeds even when the editor could not be
started. In contrast, other editor-launching paths in git (such
as "git commit" and "git rebase --edit-todo") properly propagate
editor failures and exit with an error.

Check the return value and propagate the failure by returning -1.
The two callers (cmd_config_edit at line 1315 and the legacy
cmd_config at line 1478) both propagate this return to
handle_builtin, which translates negative returns into an error
exit.

Pointed out by Coverity.

Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The object lookup machinery has been taught to gracefully recover
when a multi-pack-index points to an owning pack that was removed
during a concurrent geometric repack, and 'git replay' has been
fixed to not segfault when reading such missing objects.

* en/midx-missing-pack-fallback:
  packfile: recover when a multi-pack-index names a removed pack
  mktree: do not use OBJECT_INFO_QUICK when checking objects
  mktree: plug per-tree leak in --batch mode
  replay: fail gracefully when a merge input is unreadable
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix conjoined bullet items for the maintenance strategies in
git-config(1). Reportedly, asciidoctor renders this document correctly
both before and after this patch. asciidoc renders it correctly only
_after_ this patch.

Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our "documentation" job has recently stopped working with the following
error:

  + sudo gem install --version 1.5.8 asciidoctor
  + gem install --version 1.5.8 asciidoctor
  ./ci/install-dependencies.sh: 23: gem: not found

The root cause of this is that we never explicitly install Ruby, and
consequently gem(1) isn't explicitly pulled in, either. This used to
work alright because we transitively pulled in Ruby via asciidoc. But
due to an update it seems that we stopped pulling in the transitive
dependency, and consequently we don't have gem(1) available anymore.

Fix this by explicitly installing Ruby.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our install-dependencies script avoids installing asciidoctor if the
CI environment claims to have it already. But the only environment which
made use of this was Azure Pipelines, which went away in 6081d38 (ci:
retire the Azure Pipelines definition, 2020-04-11). So this conditional
was effectively doing nothing. Let's clean it up.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* 'master' of https://github.com/j6t/gitk:
  gitk: discourage AI contributions
  gitk: move UI for generic colors above diff colors
  gitk: avoid constructing dialog titles from text pieces
  gitk: use more natural language for labels of color preferences
  gitk: show color preferences on the button instead of the label
  gitk: condense repetitive code around color buttons into foreach loops
  gitk: set intitial colors of swatches using the available helper
The CI job for Debian 11 has been updated to use Debian 12, as the
former is now out of the LTS period.

* jk/ci-bump-debian-to-12:
  ci: bump debian-11 job to debian-12
The CI script to install dependencies for the documentation build
has been updated to install asciidoctor directly via the system
package manager instead of pinning to an older version via gem.
Additionally, an obsolete variable used for retired Azure Pipelines
environments has been removed.

* jk/ci-use-system-asciidoctor:
  ci: drop ALREADY_HAVE_ASCIIDOCTOR variable
  ci: fix missing Ruby dependency in "documentation" job
A collection of patches from Git for Windows has been upstreamed,
mostly focusing on simplifying and robustifying build configurations
for MinGW/MSYS2, dropping obsolete compatibility options, and allowing
the main 'git.exe' to be used directly without the extra wrapper
process on Windows.

* js/mingw-build-updates:
  t0060: adjust the code style
  mingw: allow `git.exe` to be used instead of the "Git wrapper"
  mingw: ensure valid CTYPE
  mingw: always define `ETC_*` for MSYS2 environments
  windows: skip linking `git-<command>` for built-ins
  mingw: rely on MSYS2's metadata instead of hard-coding it
  mingw: only enable the MSYS2-specific stuff when compiling in MSYS2
  mingw: set the prefix and HOST_CPU as per MSYS2's settings
  mingw: avoid over-specifying `--pic-executable`
  mingw: only use -Wl,--large-address-aware for 32-bit builds
  mingw: drop the -D_USE_32BIT_TIME_T option
  mingw: stop hard-coding `CC = gcc`
  mingw: include the Python parts in the build
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our CI Documentation builds have pinned asciidoctor since 615a6c3
(ci: stick with Asciidoctor v1.5.8 for now, 2019-03-29). Back then a few
changes were needed to build with the then-new v2.0.0.

We've since made those changes, in f6461b8 (Documentation: fix build
with Asciidoctor 2, 2019-09-15) and other commits. So it is not only
safe to use newer versions of asciidoctor, but preferable: it's what
people are likely to use for actually building the documentation in
practice!

I don't think there's any need to pin to a specific version. We can just
use what ships in our Ubuntu image, which should give us an arbitrary
representative version (and if it changes and something breaks, we'd
want to know).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The CI script to install dependencies for the documentation build
has been updated to install asciidoctor directly via the system
package manager instead of pinning to an older version via gem.
Additionally, an obsolete variable used for retired Azure Pipelines
environments has been removed.

* jk/ci-use-system-asciidoctor:
  ci: use system asciidoctor
Mark-up fix for 'git maintenance' documentation pages.

* bc/maintenance-doc-markup-fix-for-asciidoc:
  doc: fix conjoined maintenance strategies in git-config(1)
Teach 'am', 'revert', and 'rebase' that running 'commit --amend' or a
partial 'commit <paths>' makes no sense during operations that stop
and return control to the user to resolve conflicts left in the
working tree, just like 'cherry-pick' and 'merge' do.

* en/no-amend-during-conflicts:
  commit: refuse partial commits during conflict resolution
  commit: refuse to amend during conflict resolution
  commit: reword the empty-commit rebase amend error
  commit: allow a partial commit when a rebase pick becomes empty
  commit: clarify FROM_REBASE_PICK and is_from_rebase() names
The error path in 'git submodule--helper' has been updated to plug a
memory leak when a repository handle could not be obtained,
leveraging an updated idempotent repo_clear().

* jk/submodule-error-leak:
  submodule--helper: free URL when repository setup fails
  repository: make repo_clear() idempotent
Two members in "struct pathspec_item" were of type "char *", but
nobody updated the string through these pointers.  They have been
made "const char *" instead.

* jc/pathspec-match-const:
  pathspec: match and original in pathspec_item are const
The process of downloading packfile URIs in protocol v2 has been
instrumented with a Trace2 region.  This visibility allows tracking
the cumulative time spent downloading external packs and the number
of advertised URIs without emitting a separate event per pack.

* tn/fetch-pack-trace-packfile-uri:
  fetch-pack: trace packfile URI downloads
The gitdatamodel documentation page has been linked from a handful
of key documentaiton pages.

* kh/doc-datamodel:
  doc: datamodel: link to the glossary
  doc: glossary: link four of the terms to gitdatamodel(7)
  doc: git: link to the gitdatamodel(7) tutorial
  doc: git: list gitdatamodel(7) as a concept guide
The documentation for 'git cherry-pick' has been updated to clarify
that the '--no-commit' option intentionally skips setting the
'CHERRY_PICK_HEAD' ref.  A test has also been added to ensure this
behavior holds even when the operation stops for conflicts.

* as/cherry-pick-no-commit-doc:
  doc: cherry-pick: note --no-commit skips CHERRY_PICK_HEAD
  t3507: check no CHERRY_PICK_HEAD after conflicting --no-commit
Running "git history" in a corrupt repository can (unsurprisingly)
segfault when a necessary tree object is not found.

* jc/history-missing-tree-errorfix:
  history: do not dereference NULL when parent tree is missing
The development helper script to lint gitlink references in the
documentation has been updated to avoid a newer Perl regular
expression syntax that breaks on older Perl versions.

* ta/lint-gitlink-older-perl-fix:
  lint-gitlink: don't use empty lower bound in .{0,8}
Signed-off-by: Junio C Hamano <gitster@pobox.com>
CGI helper scripts used by HTTP-related test scripts have been updated
to use atomic filesystem operations, preventing race conditions when
Apache handles concurrent requests.

* mm/lib-httpd-cgi-safe:
  t/lib-httpd: document writing concurrency-safe CGI helpers
  t/lib-httpd: make http-429 first-request check atomic
  t/lib-httpd: fix apply-one-time-script race under concurrent requests
The 'git imap-send' command has been taught to take the '--draft'
option to mark uploaded messages as drafts, which helps some email
clients render them properly for editing and sending.

* wf/imap-send-draft:
  imap-send: add --draft to set IMAP \Draft flag
The autostash fallback in 'git checkout -m' has been refined to only
retry when there are local changes.  Additionally, a blank line now
visually separates autostash conflict advice from the subsequent
branch-switch message.

* hn/checkout-m-autostash-refine:
  checkout: separate autostash conflict advice from branch-switch message
  stash: reserve exit status 1 for conflicts
"git maintenance" triggered "rerere gc" in unappropriate times and
interfered with "git rebase" etc. too much.  The conditions "rerere
gc" gets triggered have been tweaked.

* ps/tune-rerere-gc:
  builtin/maintenance: improve heuristic for "rerere gc"
  rerere: extract logic to determine whether entries are stale
The git rev-list command has been augmented with a '--missing-only'
option that filters the output to only show missing objects,
stripping the leading '?' character and suppressing present objects,
which is useful when used in combination with '--missing=print' or
'--missing=print-info'.

* sa/rev-list-missing-only:
  rev-list: add --missing-only option to filter output
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.