Jeff King <peff@xxxxxxxx> writes: > On Thu, Jul 17, 2025 at 12:35:58PM -0700, Kyle Lippincott wrote: > >> > ==3275333==WARNING: MemorySanitizer: use-of-uninitialized-value >> > #0 0x557bd886f4bb in git_mkstemps_mode ../wrapper.c:487:27 >> > #1 0x557bd886fb55 in git_mkstemp_mode ../wrapper.c:509:9 >> > #2 0x557bd8100d1a in create_tmpfile ../object-file.c:736:7 >> > #3 0x557bd80f1630 in start_loose_object_common ../object-file.c:781:7 >> > #4 0x557bd80f5203 in write_loose_object ../object-file.c:881:7 >> > #5 0x557bd80f4875 in write_object_file_flags ../object-file.c:1086:6 >> > #6 0x557bd80f9f65 in write_object_file ../object-file.h:181:9 >> > #7 0x557bd8101eb8 in index_mem ../object-file.c:1177:9 >> > #8 0x557bd80f8bd5 in index_core ../object-file.c:1247:10 >> > #9 0x557bd80f731d in index_fd ../object-file.c:1274:9 >> > #10 0x557bd80f95e4 in index_path ../object-file.c:1295:7 >> > #11 0x557bd831132d in add_to_index ../read-cache.c:771:7 >> > #12 0x557bd8313cb1 in add_file_to_index ../read-cache.c:804:9 >> > #13 0x557bd73f892c in add_files ../builtin/add.c:355:7 >> > #14 0x557bd73f4752 in cmd_add ../builtin/add.c:578:18 >> > #15 0x557bd7a38b6f in run_builtin ../git.c:480:11 >> > #16 0x557bd7a31d54 in handle_builtin ../git.c:746:9 >> > #17 0x557bd7a36644 in run_argv ../git.c:813:4 >> > #18 0x557bd7a30e09 in cmd_main ../git.c:953:19 >> > #19 0x557bd7a3ca01 in main ../common-main.c:9:11 >> > #20 0x7f7e3f02a4d7 in __libc_start_call_main >> > (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a4d7) >> > (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) >> > #21 0x7f7e3f02a59a in __libc_start_main@GLIBC_2.2.5 >> > (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a59a) >> > (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) >> > #22 0x557bd7352b34 in _start (git+0x5db34) >> > >> > Possibly something we need to look into cleaning up. >> >> I also saw those msan issues when trying `make >> CFLAGS=-fsanitize=memory CC=clang`, but not with Google's internal >> msan build. I don't know which variable in wrapper.c:487 it's >> complaining about - you'd think it'd be `letters`, but if it's `v`, >> then that potentially comes from OpenSSL or some other library, and >> that library would also need to be built with msan (which is why it's >> such a pain to get msan builds working - EVERY library needs to be >> built with memory sanitizer). > > Yeah, presumably it is "v" from csprng_bytes(). If there are only a few > such spots, we can manually "unpoison" memory coming from libraries. On > my system, I didn't hit the case shown above but do have trouble with > bytes coming back from zlib. > > Applying this ancient patch: > > https://lore.kernel.org/git/20171004101932.pai6wzcv2eohsicr@xxxxxxxxxxxxxxxxxxxxx/ > > and building with "make SANITIZE=memory CC=clang" let me run t6302 to > completion, modulo the bug that started this thread (and which I > confirmed goes away both with MSan and valgrind with the fix Karthik > posted). > > Probably: > > diff --git a/wrapper.c b/wrapper.c > index 2f00d2ac87..6a4c1c1c29 100644 > --- a/wrapper.c > +++ b/wrapper.c > @@ -482,6 +482,8 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode) > if (csprng_bytes(&v, sizeof(v), 0) < 0) > return error_errno("unable to get random bytes for temporary file"); > > + msan_unpoison(&v, sizeof(v)); > + > /* Fill in the random bits. */ > for (i = 0; i < num_x; i++) { > filename_template[i] = letters[v % num_letters]; > > > on top of that would fix the problem you guys are seeing. I don't know > if that path leads to insanity, though. Using MSan-enabled libraries is > probably a better direction (should increase accuracy, and we don't have > to carry these manual annotations around). > I wonder if an alternate is to use '-fsanitize-ignorelist', since the MemorySanitizer is supposed to work with that too [1]. [1]: https://clang.llvm.org/docs/MemorySanitizer.html#ignorelist > -Peff
Attachment:
signature.asc
Description: PGP signature
