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@xxxxxx>
---
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
