git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 01/10] bulk-checkin: fix segfault with unsafe SHA1 backend

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 30, 2024, 14:24 UTC
Message-ID
<20241230-pks-meson-sha1-unsafe-v1-1-efb276e171f5@pks.im>
In-Reply-To
<20241230-pks-meson-sha1-unsafe-v1-0-efb276e171f5@pks.im>

In 1b9e9be8b4 (csum-file.c: use unsafe SHA-1 implementation when available, 2024-09-26) we have converted our `struct hashfile` to use the unsafe SHA1 backend, which results in a significant speedup. One needs to be careful with how to use that structure now though because callers need to consistently use either the safe or unsafe variants of SHA1, as otherwise one can easily trigger corruption.

As it turns out, we have one inconsistent usage in our tree because we directly initialize `struct hashfile_checkpoint::ctx` with the safe variant of SHA1, but end up writing to that context with the unsafe ones. This went unnoticed so far because our CI systems do not exercise different hash functions for these two backends, and consequently safe and unsafe variants are equivalent. But when using SHA1DC as safe and OpenSSL as unsafe backend this leads to a crash an t1050:

    ++ git -c core.compression=0 add large1
    AddressSanitizer:DEADLYSIGNAL
    =================================================================
    ==1367==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7ffff7a01a99 bp 0x507000000db0 sp 0x7fffffff5690 T0)
    ==1367==The signal is caused by a READ memory access.
    ==1367==Hint: address points to the zero page.
        #0 0x7ffff7a01a99 in EVP_MD_CTX_copy_ex (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4)
        #1 0x555555ddde56 in openssl_SHA1_Clone ../sha1/openssl.h:40:2
        #2 0x555555dce2fc in git_hash_sha1_clone_unsafe ../object-file.c:123:2
        #3 0x555555c2d5f8 in hashfile_checkpoint ../csum-file.c:211:2
        #4 0x555555b9905d in deflate_blob_to_pack ../bulk-checkin.c:286:4
        #5 0x555555b98ae9 in index_blob_bulk_checkin ../bulk-checkin.c:362:15
        #6 0x555555ddab62 in index_blob_stream ../object-file.c:2756:9
        #7 0x555555dda420 in index_fd ../object-file.c:2778:9
        #8 0x555555ddad76 in index_path ../object-file.c:2796:7
        #9 0x555555e947f3 in add_to_index ../read-cache.c:771:7
        #10 0x555555e954a4 in add_file_to_index ../read-cache.c:804:9
        #11 0x5555558b5c39 in add_files ../builtin/add.c:355:7
        #12 0x5555558b412e in cmd_add ../builtin/add.c:578:18
        #13 0x555555b1f493 in run_builtin ../git.c:480:11
        #14 0x555555b1bfef in handle_builtin ../git.c:740:9
        #15 0x555555b1e6f4 in run_argv ../git.c:807:4
        #16 0x555555b1b87a in cmd_main ../git.c:947:19
        #17 0x5555561649e6 in main ../common-main.c:64:11
        #18 0x7ffff742a1fb in __libc_start_call_main (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a1fb) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
        #19 0x7ffff742a2b8 in __libc_start_main@GLIBC_2.2.5 (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a2b8) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4)
        #20 0x555555772c84 in _start (git+0x21ec84)
    ==1367==Register values:
    rax = 0x0000511000001080  rbx = 0x0000000000000000  rcx = 0x000000000000000c  rdx = 0x0000000000000000
    rdi = 0x0000000000000000  rsi = 0x0000507000000db0  rbp = 0x0000507000000db0  rsp = 0x00007fffffff5690
     r8 = 0x0000000000000000   r9 = 0x0000000000000000  r10 = 0x0000000000000000  r11 = 0x00007ffff7a01a30
    r12 = 0x0000000000000000  r13 = 0x00007fffffff6b38  r14 = 0x00007ffff7ffd000  r15 = 0x00005555563b9910
    AddressSanitizer can not provide additional info.
    SUMMARY: AddressSanitizer: SEGV (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4) in EVP_MD_CTX_copy_ex
    ==1367==ABORTING
    ./test-lib.sh: line 1023:  1367 Aborted                 git $config add large1
    error: last command exited with $?=134
    not ok 4 - add with -c core.compression=0
Fix the issue by using the unsafe variant instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 bulk-checkin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 4a70a70a951cfd1a488339a33bf3a76b5152a344..433070a3bda461a2ad62da67cc5515f8822d2df3 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -272,7 +272,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
 					  OBJ_BLOB, size);
 	the_hash_algo->init_fn(&ctx);
 	the_hash_algo->update_fn(&ctx, obuf, header_len);
-	the_hash_algo->init_fn(&checkpoint.ctx);
+	the_hash_algo->unsafe_init_fn(&checkpoint.ctx);
 
 	/* Note: idx is non-NULL when we are writing */
 	if ((flags & HASH_WRITE_OBJECT) != 0)
-- 
2.48.0.rc0.311.gb6c66824c1.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 10 of 19 in “Fix segfaults when using the unsafe SHA1 backend”
  1. 00/10 Fix segfaults when using the unsafe SHA1 backendPatrick Steinhardt, Dec 30, 2024
  2. 02/10 builtin/fast-import: fix segfault with unsafe SHA1 backendPatrick Steinhardt, Dec 30, 2024
  3. Taylor BlauDec 30, 2024
  4. Patrick SteinhardtJan 3, 2025
  5. Junio C HamanoJan 3, 2025
  6. Taylor BlauJan 6, 2025
  7. Patrick SteinhardtJan 7, 2025
  8. Taylor BlauJan 8, 2025
  9. Patrick SteinhardtJan 9, 2025
  10. 01/10 bulk-checkin: fix segfault with unsafe SHA1 backendPatrick Steinhardt, Dec 30, 2024
  11. 03/10 ci: exercise unsafe OpenSSL backendPatrick Steinhardt, Dec 30, 2024
  12. Taylor BlauDec 30, 2024
  13. 04/10 meson: consistenlty spell 'CommonCrypto'Patrick Steinhardt, Dec 30, 2024
  14. 05/10 meson: deduplicate access to SHA1/SHA256 backend optionsPatrick Steinhardt, Dec 30, 2024
  15. 06/10 meson: require SecurityFramework when it's used as SHA1 backendPatrick Steinhardt, Dec 30, 2024
  16. 07/10 meson: simplify conditions for HTTPS and SHA1 dependenciesPatrick Steinhardt, Dec 30, 2024
  17. 08/10 meson: add missing dots for build optionsPatrick Steinhardt, Dec 30, 2024
  18. 09/10 meson: wire up unsafe SHA1 backendPatrick Steinhardt, Dec 30, 2024
  19. 10/10 meson: provide a summary of configured backendsPatrick Steinhardt, Dec 30, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.