{"thread":{"id":"55900","subject":"[PATCH 2/3] split-index: use oideq instead of memcmp to compare object_id's","startedAt":"2021-06-10T16:48:38Z","lastAt":"2021-06-20T15:23:10Z","messageCount":15,"participants":["Andrzej Hunt via GitGitGadget","Chris Torek","Junio C Hamano","Andrzej Hunt","Jeff King","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"427040","messageId":"14b0d5dd7fce086dd7855f3bcd1b782eb237a682.1623343713.git.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":"pull.1033.git.git.1623343712.gitgitgadget@gmail.com","subject":"[PATCH 2/3] split-index: use oideq instead of memcmp to compare object_id's","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-10T16:48:31Z","receivedAt":"2021-06-10T16:48:38Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt \n\ncache_entry contains an object_id, and compare_ce_content() would\ninclude that field when calling memcmp on a subset of the cache_entry.\nDepending on which hashing algorithm is being used, only part of\nobject_id.hash is actually being used, therefore including it in a\nmemcmp() is technically incorrect. Instead we choose to exclude the\nobject_id when calling memcmp(), and call oideq() separately.\n\nThis issue was found when running t1700-split-index with MSAN, see MSAN\noutput below (on my machine, offset 76 corresponds to 4 bytes after the\nstart of object_id.hash).\n\nUninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)\n==27914==WARNING: MemorySanitizer: use-of-uninitialized-value\n #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10\n #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8\n #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9\n #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2\n #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8\n #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7\n #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9\n #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3\n #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2\n #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18\n #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2\n #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11\n #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12\n #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12\n #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6\n #8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17\n #9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9\n #10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8\n #11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7\n #12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2\n #13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12\n #14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9\n #15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n\n Uninitialized value was created by a heap allocation\n #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3\n #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8\n #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9\n #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6\n #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3\n #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c\n #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17\n #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8\n #8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8\n #9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2\n #10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6\n #11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp\nExiting\n\nSigned-off-by: Andrzej Hunt \n---\n split-index.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/split-index.c b/split-index.c\nindex 4d6e52d46f75..8e52e891c3bc 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -207,7 +207,8 @@ static int compare_ce_content(struct cache_entry *a, struct cache_entry *b)\n \tb->ce_flags &= ondisk_flags;\n \tret = memcmp(&a->ce_stat_data, &b->ce_stat_data,\n \t\t offsetof(struct cache_entry, name) -\n-\t\t offsetof(struct cache_entry, ce_stat_data));\n+\t\t offsetof(struct cache_entry, oid)) ||\n+\t\t!oideq(&a->oid, &b->oid);\n \ta->ce_flags = ce_flags;\n \tb->ce_flags = base_flags;\n \n-- \ngitgitgadget\n\n"},{"id":"427041","messageId":"cd1e1f6985c77d21ec869e53dc5eb79673caf491.1623343713.git.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":"pull.1033.git.git.1623343712.gitgitgadget@gmail.com","subject":"[PATCH 3/3] builtin/checkout--worker: memset struct to avoid MSAN complaints","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-10T16:48:32Z","receivedAt":"2021-06-10T16:48:52Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt \n\nreport_result() sends a struct to the parent process, but that struct\ncontains unintialised padding bytes. Running this code under MSAN\nrightly triggers a warning - but we also don't care about this warning\nbecause we control the receiving code, and we therefore know that those\npadding bytes won't be read on the receiving end. Therefore we add a\nmemset to convince MSAN that this memory is safe to read - but only\nwhen building with MSAN to avoid this cost in normal usage.\n\nInterestingly, in the error-case branch, we only try to copy the first\ntwo members of pc_item_result, by copying only PC_ITEM_RESULT_BASE_SIZE\nbytes. However PC_ITEM_RESULT_BASE_SIZE is defined as\n'offsetof(the_last_member)', which means that we're copying padding bytes\nafter the end of the second last member. We could avoid doing this by\nredefining PC_ITEM_RESULT_BASE_SIZE as\n'offsetof(second_last_member) + sizeof(second_last_member)', but there's\nno huge benefit to doing so (and our memset hack silences the MSAN\nwarning in this scenario either way).\n\nMSAN output from t2080 (partially interleaved due to the\nparallel work :) ):\n\nUninitialized bytes in __interceptor_write at offset 12 inside [0x7fff37d83408, 160)\n==23279==WARNING: MemorySanitizer: use-of-uninitialized-value\nUninitialized bytes in __interceptor_write at offset 12 inside [0x7ffdb8a07ec8, 160)\n==23280==WARNING: MemorySanitizer: use-of-uninitialized-value\n #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8\n #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21\n #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6\n #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6\n #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2\n #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3\n #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2\n #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #12 0x7f8778114349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120\n\n Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result'\n #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite\nExiting\n #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8\n #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21\n #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6\n #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6\n #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2\n #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3\n #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2\n #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #12 0x7f2749a0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120\n\n Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result'\n #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite\n\nSigned-off-by: Andrzej Hunt \n---\n builtin/checkout--worker.c | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/builtin/checkout--worker.c b/builtin/checkout--worker.c\nindex 289a9b8f89d0..02fa5285988f 100644\n--- a/builtin/checkout--worker.c\n+++ b/builtin/checkout--worker.c\n@@ -56,6 +56,17 @@ static void report_result(struct parallel_checkout_item *pc_item)\n \tstruct pc_item_result res;\n \tsize_t size;\n \n+#if defined(__has_feature)\n+# if __has_feature(memory_sanitizer)\n+\t// MSAN workaround: res contains padding bytes, which will remain\n+\t// permanently unintialised. Later, we read all of res in order to send\n+\t// it to the parent process - and MSAN (rightly) complains that we're\n+\t// reading those unintialised padding bytes. By memset'ing res we\n+\t// guarantee that there are no uninitialised bytes.\n+\tmemset(&res, 0, sizeof(res));\n+#endif\n+#endif\n+\n \tres.id = pc_item->id;\n \tres.status = pc_item->status;\n \n-- \ngitgitgadget\n"},{"id":"427042","messageId":"pull.1033.git.git.1623343712.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":null,"subject":"[PATCH 0/3] Fix uninitialised reads found with MSAN","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-10T16:48:29Z","receivedAt":"2021-06-10T16:49:36Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"This series fixes a small number of issues found when running git's\ntest-suite with MSAN (MemorySanitizer: a clang sanitizer that tries to\ndetect reads from uninitialised memory [2]). To summarise: I think there's\none real bug, one theoretical bug where compiler nevertheless produce\nworking code, and one false-positive that we can easily suppress.\n\nGetting the test suite to run under MSAN is a bit trickier than simply\nadding SANITIZERS=memory, I've detailed the reasons and the process I'm\nusing below. Unfortunately this series is also not sufficient to make the\nwhole test suite pass when building with MSAN:\n\n * t0005-sigchain and t7006-pager fail with an infinite loop inside MSAN's\n signal handling interceptors. I think this is a bad interaction between\n git's signal handling and MSAN's interceptors, and I suspect it's not\n indicative of a bug in git itself - but I haven't investigated in detail\n yet.\n * t3206-range-diff, t4013-diff-various, t4018-diff-funcname all fail due to\n a change in diff output. I can reproduce this issue when running with\n TSAN (but not ASAN or UBSAN), which suggests a bug or difference in\n behaviour in code shared between MSAN and TSAN - similarly, I haven't\n investigated in all that much detail yet.\n\n(These issues were seen when running with clang-11 - the next step is to\ntest with clang built from main)\n\nAs to the tricky part: MSAN tries to detect reads from uninitialised memory\nat runtime. However you need to ensure that all code performing\ninitialisation is built with the right instrumentation (i.e.\n-fsanitize=memory). So you'll immediately run into issues if you link\nagainst libraries provided by your system (with the exception of libc, as\nMSAN provides some default interceptors for most of libc). In theory you\nshould rebuild all dependencies with -fsanitize=memory, although I\ndiscovered that it's sufficient to recompile only zlib + link git against\nthat copy of zlib (which not a very tricky thing to do). Doing this will\nuncover one intentional read from uninitialised memory inside zlib itself.\nThis can be worked around with an annotation in zlib (which I'm trying to\nsubmit upstream at [1]) - but it's also possible to define an override list\nat compile time - I've detailed this in my recipe below).\n\nMy recipe for running git tests against MSAN:\n\n 1. Grab zlib sources from zlib.net or github.com/madler/zlib , I used zlib\n 1.2.11 (which is also what most systems seem to ship).\n\n 2. Create a sanitizers special cast list (named e.g. ignorelist.txt)\n containing \"fun:slide_hash\" (this is only needed as long as zlib doesn't\n contain [1]).\n\n 3. Build zlib, installing it into SOME_PREFIX (I happened to use clang, but\n that might not be necessary): CC=clang-11 CFLAGS=\"-fsanitize=memory\n -fno-sanitize-recover=memory\n -fsanitize-ignorelist=YOUR_IGNORELIST_FROM_STEP_2\" ./configure && make\n install prefix=$SOME_PREFIX\n\n 4. Build git and run the tests (again, I'm using clang, but gcc might be OK\n too): make ZLIB_PATH=$SOME_PREFIX CC=clang-11 SANITIZERS=memory test\n\nIf you're actively trying to understand and fix issues, I also recommend\nadding -fsanitize-memory-track-origins (which points you directly to where\nthe uninitialised memory comes from), see also further docs at [2].\n\nATB,\n\nAndrzej\n\n[1] https://github.com/madler/zlib/pull/561\n\n[2] https://clang.llvm.org/docs/MemorySanitizer.html\n\nAndrzej Hunt (3):\n bulk-checkin: make buffer reuse more obvious and safer\n split-index: use oideq instead of memcmp to compare object_id's\n builtin/checkout--worker: memset struct to avoid MSAN complaints\n\n builtin/checkout--worker.c | 11 +++++++++++\n bulk-checkin.c | 3 +--\n split-index.c | 3 ++-\n 3 files changed, 14 insertions(+), 3 deletions(-)\n\n\nbase-commit: 62a8d224e6203d9d3d2d1d63a01cf5647ec312c9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1033%2Fahunt%2Fmsan-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1033/ahunt/msan-v1\nPull-Request: https://github.com/git/git/pull/1033\n-- \ngitgitgadget\n"},{"id":"427043","messageId":"7659d4bf13c27ed0b1b793a19959b469063b85ec.1623343712.git.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":"pull.1033.git.git.1623343712.gitgitgadget@gmail.com","subject":"[PATCH 1/3] bulk-checkin: make buffer reuse more obvious and safer","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-10T16:48:30Z","receivedAt":"2021-06-10T16:49:49Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt \n\nibuf can be reused for multiple iterations of the loop. Specifically:\ndeflate() overwrites s.avail_in to show how much of the input buffer\nhas not been processed yet - and sometimes leaves 'avail_in > 0', in\nwhich case ibuf will be processed again during the loop's subsequent\niteration.\n\nBut if we declare ibuf within the loop, then (in theory) we get a new\n(and uninitialised) buffer for every iteration. In practice, my compiler\nseems to resue the same buffer - meaning that this code does work - but\nit doesn't seem safe to rely on this behaviour. MSAN correctly catches\nthis issue - as soon as we hit the 's.avail_in > 0' condition, we end up\nreading from what seems to be uninitialised memory.\n\nTherefore, we move ibuf out of the loop, making this reuse safe.\n\nSee MSAN output from t1050-large below - the interesting part is the\nibuf creation at the end, although there's a lot of indirection before\nwe reach the read from unitialised memory:\n\n==11294==WARNING: MemorySanitizer: use-of-uninitialized-value\n #0 0x7f75db58fb1c in crc32_little crc32.c:283:9\n #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20\n #2 0x7f75db59668c in crc32 crc32.c:242:12\n #3 0x8c94f8 in hashwrite csum-file.c:101:15\n #4 0x825faf in stream_to_pack bulk-checkin.c:154:5\n #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #7 0xa7cff2 in index_stream object-file.c:2234:9\n #8 0xa7bff7 in index_fd object-file.c:2256:9\n #9 0xa7d22d in index_path object-file.c:2274:7\n #10 0xb3c8c9 in add_to_index read-cache.c:802:7\n #11 0xb3e039 in add_file_to_index read-cache.c:835:9\n #12 0x4a99c3 in add_files add.c:458:7\n #13 0x4a7276 in cmd_add add.c:670:18\n #14 0x4a1e76 in run_builtin git.c:461:11\n #15 0x49e1e7 in handle_builtin git.c:714:3\n #16 0x4a0c08 in run_argv git.c:781:4\n #17 0x49d5a8 in cmd_main git.c:912:19\n #18 0x7974da in main common-main.c:52:11\n #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #20 0x421bd9 in _start start.S:120\n\n Uninitialized value was stored to memory at\n #0 0x7f75db58fa6b in crc32_little crc32.c:283:9\n #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20\n #2 0x7f75db59668c in crc32 crc32.c:242:12\n #3 0x8c94f8 in hashwrite csum-file.c:101:15\n #4 0x825faf in stream_to_pack bulk-checkin.c:154:5\n #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #7 0xa7cff2 in index_stream object-file.c:2234:9\n #8 0xa7bff7 in index_fd object-file.c:2256:9\n #9 0xa7d22d in index_path object-file.c:2274:7\n #10 0xb3c8c9 in add_to_index read-cache.c:802:7\n #11 0xb3e039 in add_file_to_index read-cache.c:835:9\n #12 0x4a99c3 in add_files add.c:458:7\n #13 0x4a7276 in cmd_add add.c:670:18\n #14 0x4a1e76 in run_builtin git.c:461:11\n #15 0x49e1e7 in handle_builtin git.c:714:3\n #16 0x4a0c08 in run_argv git.c:781:4\n #17 0x49d5a8 in cmd_main git.c:912:19\n #18 0x7974da in main common-main.c:52:11\n #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db5c2011 in flush_pending deflate.c:746:5\n #2 0x7f75db5cafa0 in deflate_stored deflate.c:1815:9\n #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #4 0xd80b7f in git_deflate zlib.c:244:12\n #5 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #8 0xa7cff2 in index_stream object-file.c:2234:9\n #9 0xa7bff7 in index_fd object-file.c:2256:9\n #10 0xa7d22d in index_path object-file.c:2274:7\n #11 0xb3c8c9 in add_to_index read-cache.c:802:7\n #12 0xb3e039 in add_file_to_index read-cache.c:835:9\n #13 0x4a99c3 in add_files add.c:458:7\n #14 0x4a7276 in cmd_add add.c:670:18\n #15 0x4a1e76 in run_builtin git.c:461:11\n #16 0x49e1e7 in handle_builtin git.c:714:3\n #17 0x4a0c08 in run_argv git.c:781:4\n #18 0x49d5a8 in cmd_main git.c:912:19\n #19 0x7974da in main common-main.c:52:11\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db644241 in _tr_stored_block trees.c:873:5\n #2 0x7f75db5cad7c in deflate_stored deflate.c:1813:9\n #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #4 0xd80b7f in git_deflate zlib.c:244:12\n #5 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #8 0xa7cff2 in index_stream object-file.c:2234:9\n #9 0xa7bff7 in index_fd object-file.c:2256:9\n #10 0xa7d22d in index_path object-file.c:2274:7\n #11 0xb3c8c9 in add_to_index read-cache.c:802:7\n #12 0xb3e039 in add_file_to_index read-cache.c:835:9\n #13 0x4a99c3 in add_files add.c:458:7\n #14 0x4a7276 in cmd_add add.c:670:18\n #15 0x4a1e76 in run_builtin git.c:461:11\n #16 0x49e1e7 in handle_builtin git.c:714:3\n #17 0x4a0c08 in run_argv git.c:781:4\n #18 0x49d5a8 in cmd_main git.c:912:19\n #19 0x7974da in main common-main.c:52:11\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db5c8fcf in deflate_stored deflate.c:1783:9\n #2 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #3 0xd80b7f in git_deflate zlib.c:244:12\n #4 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #7 0xa7cff2 in index_stream object-file.c:2234:9\n #8 0xa7bff7 in index_fd object-file.c:2256:9\n #9 0xa7d22d in index_path object-file.c:2274:7\n #10 0xb3c8c9 in add_to_index read-cache.c:802:7\n #11 0xb3e039 in add_file_to_index read-cache.c:835:9\n #12 0x4a99c3 in add_files add.c:458:7\n #13 0x4a7276 in cmd_add add.c:670:18\n #14 0x4a1e76 in run_builtin git.c:461:11\n #15 0x49e1e7 in handle_builtin git.c:714:3\n #16 0x4a0c08 in run_argv git.c:781:4\n #17 0x49d5a8 in cmd_main git.c:912:19\n #18 0x7974da in main common-main.c:52:11\n #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db5ea545 in read_buf deflate.c:1181:5\n #2 0x7f75db5c97f7 in deflate_stored deflate.c:1791:9\n #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #4 0xd80b7f in git_deflate zlib.c:244:12\n #5 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #8 0xa7cff2 in index_stream object-file.c:2234:9\n #9 0xa7bff7 in index_fd object-file.c:2256:9\n #10 0xa7d22d in index_path object-file.c:2274:7\n #11 0xb3c8c9 in add_to_index read-cache.c:802:7\n #12 0xb3e039 in add_file_to_index read-cache.c:835:9\n #13 0x4a99c3 in add_files add.c:458:7\n #14 0x4a7276 in cmd_add add.c:670:18\n #15 0x4a1e76 in run_builtin git.c:461:11\n #16 0x49e1e7 in handle_builtin git.c:714:3\n #17 0x4a0c08 in run_argv git.c:781:4\n #18 0x49d5a8 in cmd_main git.c:912:19\n #19 0x7974da in main common-main.c:52:11\n\n Uninitialized value was created by an allocation of 'ibuf' in the stack frame of function 'stream_to_pack'\n #0 0x825710 in stream_to_pack bulk-checkin.c:101\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value crc32.c:283:9 in crc32_little\nExiting\n\nSigned-off-by: Andrzej Hunt \n---\n bulk-checkin.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex 127312acd1ed..b023d9959aae 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -100,6 +100,7 @@ static int stream_to_pack(struct bulk_checkin_state *state,\n \t\t\t const char *path, unsigned flags)\n {\n \tgit_zstream s;\n+\tunsigned char ibuf[16384];\n \tunsigned char obuf[16384];\n \tunsigned hdrlen;\n \tint status = Z_OK;\n@@ -113,8 +114,6 @@ static int stream_to_pack(struct bulk_checkin_state *state,\n \ts.avail_out = sizeof(obuf) - hdrlen;\n \n \twhile (status != Z_STREAM_END) {\n-\t\tunsigned char ibuf[16384];\n-\n \t\tif (size && !s.avail_in) {\n \t\t\tssize_t rsize = size < sizeof(ibuf) ? size : sizeof(ibuf);\n \t\t\tssize_t read_result = read_in_full(fd, ibuf, rsize);\n-- \ngitgitgadget\n\n"},{"id":"427081","messageId":"CAPx1GvfF7NZD4RMWWZCfny2gQOt1GY=TUm1HxvC+kwwDsc4Rgw@mail.gmail.com","threadId":"55900","inReplyTo":"cd1e1f6985c77d21ec869e53dc5eb79673caf491.1623343713.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] builtin/checkout--worker: memset struct to avoid MSAN complaints","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2021-06-11T04:43:27Z","receivedAt":"2021-06-11T04:43:58Z","isPatch":true,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Thu, Jun 10, 2021 at 9:49 AM Andrzej Hunt via GitGitGadget\n wrote:\n> [snip] Therefore we add a\n> memset to convince MSAN that this memory is safe to read - but only\n> when building with MSAN to avoid this cost in normal usage.\n\nIt does not seem likely to be that expensive, and would definitely\nbe shorter without all the `#if` testing:\n\n> diff --git a/builtin/checkout--worker.c b/builtin/checkout--worker.c\n> index 289a9b8f89d0..02fa5285988f 100644\n> --- a/builtin/checkout--worker.c\n> +++ b/builtin/checkout--worker.c\n> @@ -56,6 +56,17 @@ static void report_result(struct parallel_checkout_item *pc_item)\n> struct pc_item_result res;\n\nThis could just have `= { 0 }` added.\n\nIn any case, this and all the others in this series look good to me.\n\nChris\n"},{"id":"427085","messageId":"xmqq7dj0530s.fsf@gitster.g","threadId":"55900","inReplyTo":"CAPx1GvfF7NZD4RMWWZCfny2gQOt1GY=TUm1HxvC+kwwDsc4Rgw@mail.gmail.com","subject":"Re: [PATCH 3/3] builtin/checkout--worker: memset struct to avoid MSAN complaints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-11T06:28:03Z","receivedAt":"2021-06-11T06:28:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Torek writes:\n\n> On Thu, Jun 10, 2021 at 9:49 AM Andrzej Hunt via GitGitGadget\n> wrote:\n>> [snip] Therefore we add a\n>> memset to convince MSAN that this memory is safe to read - but only\n>> when building with MSAN to avoid this cost in normal usage.\n>\n> It does not seem likely to be that expensive, and would definitely\n> be shorter without all the `#if` testing:\n>\n>> diff --git a/builtin/checkout--worker.c b/builtin/checkout--worker.c\n>> index 289a9b8f89d0..02fa5285988f 100644\n>> --- a/builtin/checkout--worker.c\n>> +++ b/builtin/checkout--worker.c\n>> @@ -56,6 +56,17 @@ static void report_result(struct parallel_checkout_item *pc_item)\n>> struct pc_item_result res;\n>\n> This could just have `= { 0 }` added.\n\nI'd prefer that very much more than the #if testing, within which //\ncomments that we do not want in our codebase are enclosed.\n\nThanks.\n\n\n"},{"id":"427116","messageId":"5a7a57d8-3704-4531-44f5-4d0d48ec376c@ahunt.org","threadId":"55900","inReplyTo":"xmqq7dj0530s.fsf@gitster.g","subject":"Re: [PATCH 3/3] builtin/checkout--worker: memset struct to avoid MSAN complaints","fromName":"Andrzej Hunt","fromEmail":"andrzej@ahunt.org","sentAt":"2021-06-11T15:37:40Z","receivedAt":"2021-06-11T15:37:56Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"\n\nOn 11/06/2021 08:28, Junio C Hamano wrote:\n> Chris Torek writes:\n> \n>> On Thu, Jun 10, 2021 at 9:49 AM Andrzej Hunt via GitGitGadget\n>> wrote:\n>>> [snip] Therefore we add a\n>>> memset to convince MSAN that this memory is safe to read - but only\n>>> when building with MSAN to avoid this cost in normal usage.\n>>\n>> It does not seem likely to be that expensive, and would definitely\n>> be shorter without all the `#if` testing:\n>>\n>>> diff --git a/builtin/checkout--worker.c b/builtin/checkout--worker.c\n>>> index 289a9b8f89d0..02fa5285988f 100644\n>>> --- a/builtin/checkout--worker.c\n>>> +++ b/builtin/checkout--worker.c\n>>> @@ -56,6 +56,17 @@ static void report_result(struct parallel_checkout_item *pc_item)\n>>> struct pc_item_result res;\n>>\n>> This could just have `= { 0 }` added.\n> \n> I'd prefer that very much more than the #if testing, within which //\n> comments that we do not want in our codebase are enclosed.\n\n\nI'll fix this for V2 - thanks Chris and Junio!\n\n(At the time I wasn't aware that this would include all members and \npadding, but I've learned more since reading the clang developer's \ndiscussion around padding and brace intialisation :) : \nhttps://reviews.llvm.org/D61280 . )\n"},{"id":"427129","messageId":"YMOZNyzWZbDvtFkD@coredump.intra.peff.net","threadId":"55900","inReplyTo":"pull.1033.git.git.1623343712.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Fix uninitialised reads found with MSAN","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-11T17:11:19Z","receivedAt":"2021-06-11T17:11:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 10, 2021 at 04:48:29PM +0000, Andrzej Hunt via GitGitGadget wrote:\n\n> As to the tricky part: MSAN tries to detect reads from uninitialised memory\n> at runtime. However you need to ensure that all code performing\n> initialisation is built with the right instrumentation (i.e.\n> -fsanitize=memory). So you'll immediately run into issues if you link\n> against libraries provided by your system (with the exception of libc, as\n> MSAN provides some default interceptors for most of libc). In theory you\n> should rebuild all dependencies with -fsanitize=memory, although I\n> discovered that it's sufficient to recompile only zlib + link git against\n> that copy of zlib (which not a very tricky thing to do). Doing this will\n> uncover one intentional read from uninitialised memory inside zlib itself.\n> This can be worked around with an annotation in zlib (which I'm trying to\n> submit upstream at [1]) - but it's also possible to define an override list\n> at compile time - I've detailed this in my recipe below).\n\nI played with MSAN a while ago, and yeah, the trickiest part is dealing\nwith libraries. I came up with this patch for handling zlib from within\nGit itself:\n\n https://lore.kernel.org/git/20171004101932.pai6wzcv2eohsicr@sigill.intra.peff.net/\n\nIt's entirely possible that it papers over actual bugs (perhaps even the\none your first patch is addressing). But I wonder if it's easier to\nconvince people to try the tool if there's an easy way to do it without\nrecompiling dependencies (I also hit issues with pcre and the libc\nregex; that was a few years ago, though, so I would not be at all\nsurprised if they know intercept the system regex routines, at least).\n\n-Peff\n"},{"id":"427256","messageId":"xmqqk0mx1ckt.fsf@gitster.g","threadId":"55900","inReplyTo":"5a7a57d8-3704-4531-44f5-4d0d48ec376c@ahunt.org","subject":"Re: [PATCH 3/3] builtin/checkout--worker: memset struct to avoid MSAN complaints","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-14T01:04:18Z","receivedAt":"2021-06-14T01:04:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrzej Hunt writes:\n\n> (At the time I wasn't aware that this would include all members and\n> padding, but I've learned more since reading the clang developer's \n> discussion around padding and brace intialisation :) :\n> https://reviews.llvm.org/D61280 . )\n\nThanks for a pointer ;-)\n"},{"id":"427359","messageId":"7659d4bf13c27ed0b1b793a19959b469063b85ec.1623685877.git.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":"pull.1033.v2.git.git.1623685877.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] bulk-checkin: make buffer reuse more obvious and safer","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-14T15:51:14Z","receivedAt":"2021-06-14T15:51:22Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt \n\nibuf can be reused for multiple iterations of the loop. Specifically:\ndeflate() overwrites s.avail_in to show how much of the input buffer\nhas not been processed yet - and sometimes leaves 'avail_in > 0', in\nwhich case ibuf will be processed again during the loop's subsequent\niteration.\n\nBut if we declare ibuf within the loop, then (in theory) we get a new\n(and uninitialised) buffer for every iteration. In practice, my compiler\nseems to resue the same buffer - meaning that this code does work - but\nit doesn't seem safe to rely on this behaviour. MSAN correctly catches\nthis issue - as soon as we hit the 's.avail_in > 0' condition, we end up\nreading from what seems to be uninitialised memory.\n\nTherefore, we move ibuf out of the loop, making this reuse safe.\n\nSee MSAN output from t1050-large below - the interesting part is the\nibuf creation at the end, although there's a lot of indirection before\nwe reach the read from unitialised memory:\n\n==11294==WARNING: MemorySanitizer: use-of-uninitialized-value\n #0 0x7f75db58fb1c in crc32_little crc32.c:283:9\n #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20\n #2 0x7f75db59668c in crc32 crc32.c:242:12\n #3 0x8c94f8 in hashwrite csum-file.c:101:15\n #4 0x825faf in stream_to_pack bulk-checkin.c:154:5\n #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #7 0xa7cff2 in index_stream object-file.c:2234:9\n #8 0xa7bff7 in index_fd object-file.c:2256:9\n #9 0xa7d22d in index_path object-file.c:2274:7\n #10 0xb3c8c9 in add_to_index read-cache.c:802:7\n #11 0xb3e039 in add_file_to_index read-cache.c:835:9\n #12 0x4a99c3 in add_files add.c:458:7\n #13 0x4a7276 in cmd_add add.c:670:18\n #14 0x4a1e76 in run_builtin git.c:461:11\n #15 0x49e1e7 in handle_builtin git.c:714:3\n #16 0x4a0c08 in run_argv git.c:781:4\n #17 0x49d5a8 in cmd_main git.c:912:19\n #18 0x7974da in main common-main.c:52:11\n #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #20 0x421bd9 in _start start.S:120\n\n Uninitialized value was stored to memory at\n #0 0x7f75db58fa6b in crc32_little crc32.c:283:9\n #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20\n #2 0x7f75db59668c in crc32 crc32.c:242:12\n #3 0x8c94f8 in hashwrite csum-file.c:101:15\n #4 0x825faf in stream_to_pack bulk-checkin.c:154:5\n #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #7 0xa7cff2 in index_stream object-file.c:2234:9\n #8 0xa7bff7 in index_fd object-file.c:2256:9\n #9 0xa7d22d in index_path object-file.c:2274:7\n #10 0xb3c8c9 in add_to_index read-cache.c:802:7\n #11 0xb3e039 in add_file_to_index read-cache.c:835:9\n #12 0x4a99c3 in add_files add.c:458:7\n #13 0x4a7276 in cmd_add add.c:670:18\n #14 0x4a1e76 in run_builtin git.c:461:11\n #15 0x49e1e7 in handle_builtin git.c:714:3\n #16 0x4a0c08 in run_argv git.c:781:4\n #17 0x49d5a8 in cmd_main git.c:912:19\n #18 0x7974da in main common-main.c:52:11\n #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db5c2011 in flush_pending deflate.c:746:5\n #2 0x7f75db5cafa0 in deflate_stored deflate.c:1815:9\n #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #4 0xd80b7f in git_deflate zlib.c:244:12\n #5 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #8 0xa7cff2 in index_stream object-file.c:2234:9\n #9 0xa7bff7 in index_fd object-file.c:2256:9\n #10 0xa7d22d in index_path object-file.c:2274:7\n #11 0xb3c8c9 in add_to_index read-cache.c:802:7\n #12 0xb3e039 in add_file_to_index read-cache.c:835:9\n #13 0x4a99c3 in add_files add.c:458:7\n #14 0x4a7276 in cmd_add add.c:670:18\n #15 0x4a1e76 in run_builtin git.c:461:11\n #16 0x49e1e7 in handle_builtin git.c:714:3\n #17 0x4a0c08 in run_argv git.c:781:4\n #18 0x49d5a8 in cmd_main git.c:912:19\n #19 0x7974da in main common-main.c:52:11\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db644241 in _tr_stored_block trees.c:873:5\n #2 0x7f75db5cad7c in deflate_stored deflate.c:1813:9\n #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #4 0xd80b7f in git_deflate zlib.c:244:12\n #5 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #8 0xa7cff2 in index_stream object-file.c:2234:9\n #9 0xa7bff7 in index_fd object-file.c:2256:9\n #10 0xa7d22d in index_path object-file.c:2274:7\n #11 0xb3c8c9 in add_to_index read-cache.c:802:7\n #12 0xb3e039 in add_file_to_index read-cache.c:835:9\n #13 0x4a99c3 in add_files add.c:458:7\n #14 0x4a7276 in cmd_add add.c:670:18\n #15 0x4a1e76 in run_builtin git.c:461:11\n #16 0x49e1e7 in handle_builtin git.c:714:3\n #17 0x4a0c08 in run_argv git.c:781:4\n #18 0x49d5a8 in cmd_main git.c:912:19\n #19 0x7974da in main common-main.c:52:11\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db5c8fcf in deflate_stored deflate.c:1783:9\n #2 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #3 0xd80b7f in git_deflate zlib.c:244:12\n #4 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #7 0xa7cff2 in index_stream object-file.c:2234:9\n #8 0xa7bff7 in index_fd object-file.c:2256:9\n #9 0xa7d22d in index_path object-file.c:2274:7\n #10 0xb3c8c9 in add_to_index read-cache.c:802:7\n #11 0xb3e039 in add_file_to_index read-cache.c:835:9\n #12 0x4a99c3 in add_files add.c:458:7\n #13 0x4a7276 in cmd_add add.c:670:18\n #14 0x4a1e76 in run_builtin git.c:461:11\n #15 0x49e1e7 in handle_builtin git.c:714:3\n #16 0x4a0c08 in run_argv git.c:781:4\n #17 0x49d5a8 in cmd_main git.c:912:19\n #18 0x7974da in main common-main.c:52:11\n #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3\n #1 0x7f75db5ea545 in read_buf deflate.c:1181:5\n #2 0x7f75db5c97f7 in deflate_stored deflate.c:1791:9\n #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34\n #4 0xd80b7f in git_deflate zlib.c:244:12\n #5 0x825dff in stream_to_pack bulk-checkin.c:140:12\n #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8\n #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15\n #8 0xa7cff2 in index_stream object-file.c:2234:9\n #9 0xa7bff7 in index_fd object-file.c:2256:9\n #10 0xa7d22d in index_path object-file.c:2274:7\n #11 0xb3c8c9 in add_to_index read-cache.c:802:7\n #12 0xb3e039 in add_file_to_index read-cache.c:835:9\n #13 0x4a99c3 in add_files add.c:458:7\n #14 0x4a7276 in cmd_add add.c:670:18\n #15 0x4a1e76 in run_builtin git.c:461:11\n #16 0x49e1e7 in handle_builtin git.c:714:3\n #17 0x4a0c08 in run_argv git.c:781:4\n #18 0x49d5a8 in cmd_main git.c:912:19\n #19 0x7974da in main common-main.c:52:11\n\n Uninitialized value was created by an allocation of 'ibuf' in the stack frame of function 'stream_to_pack'\n #0 0x825710 in stream_to_pack bulk-checkin.c:101\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value crc32.c:283:9 in crc32_little\nExiting\n\nSigned-off-by: Andrzej Hunt \n---\n bulk-checkin.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex 127312acd1ed..b023d9959aae 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -100,6 +100,7 @@ static int stream_to_pack(struct bulk_checkin_state *state,\n \t\t\t const char *path, unsigned flags)\n {\n \tgit_zstream s;\n+\tunsigned char ibuf[16384];\n \tunsigned char obuf[16384];\n \tunsigned hdrlen;\n \tint status = Z_OK;\n@@ -113,8 +114,6 @@ static int stream_to_pack(struct bulk_checkin_state *state,\n \ts.avail_out = sizeof(obuf) - hdrlen;\n \n \twhile (status != Z_STREAM_END) {\n-\t\tunsigned char ibuf[16384];\n-\n \t\tif (size && !s.avail_in) {\n \t\t\tssize_t rsize = size < sizeof(ibuf) ? size : sizeof(ibuf);\n \t\t\tssize_t read_result = read_in_full(fd, ibuf, rsize);\n-- \ngitgitgadget\n\n"},{"id":"427360","messageId":"6943eb511bee8323eb65f1466634ce9307694796.1623685877.git.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":"pull.1033.v2.git.git.1623685877.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] split-index: use oideq instead of memcmp to compare object_id's","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-14T15:51:15Z","receivedAt":"2021-06-14T15:51:23Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt \n\ncache_entry contains an object_id, and compare_ce_content() would\ninclude that field when calling memcmp on a subset of the cache_entry.\nDepending on which hashing algorithm is being used, only part of\nobject_id.hash is actually being used, therefore including it in a\nmemcmp() is incorrect. Instead we choose to exclude the object_id when\ncalling memcmp(), and call oideq() separately.\n\nThis issue was found when running t1700-split-index with MSAN, see MSAN\noutput below (on my machine, offset 76 corresponds to 4 bytes after the\nstart of object_id.hash).\n\nUninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)\n==27914==WARNING: MemorySanitizer: use-of-uninitialized-value\n #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10\n #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8\n #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9\n #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2\n #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8\n #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7\n #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9\n #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120\n\n Uninitialized value was stored to memory at\n #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3\n #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2\n #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18\n #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2\n #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11\n #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12\n #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12\n #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6\n #8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17\n #9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9\n #10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8\n #11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7\n #12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2\n #13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12\n #14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9\n #15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n\n Uninitialized value was created by a heap allocation\n #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3\n #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8\n #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9\n #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6\n #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3\n #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c\n #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17\n #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8\n #8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8\n #9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2\n #10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6\n #11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp\nExiting\n\nSigned-off-by: Andrzej Hunt \n---\n split-index.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/split-index.c b/split-index.c\nindex 4d6e52d46f75..8e52e891c3bc 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -207,7 +207,8 @@ static int compare_ce_content(struct cache_entry *a, struct cache_entry *b)\n \tb->ce_flags &= ondisk_flags;\n \tret = memcmp(&a->ce_stat_data, &b->ce_stat_data,\n \t\t offsetof(struct cache_entry, name) -\n-\t\t offsetof(struct cache_entry, ce_stat_data));\n+\t\t offsetof(struct cache_entry, oid)) ||\n+\t\t!oideq(&a->oid, &b->oid);\n \ta->ce_flags = ce_flags;\n \tb->ce_flags = base_flags;\n \n-- \ngitgitgadget\n\n"},{"id":"427361","messageId":"4bdc0b77f6f2a379c229b221460558ea8def5ad1.1623685877.git.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":"pull.1033.v2.git.git.1623685877.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] builtin/checkout--worker: zero-initialise struct to avoid MSAN complaints","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-14T15:51:16Z","receivedAt":"2021-06-14T15:51:25Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"From: Andrzej Hunt \n\nreport_result() sends a struct to the parent process, but that struct\nwould contain uninitialised padding bytes. Running this code under MSAN\nrightly triggers a warning - but we don't particularly care about this\nwarning because we control the receiving code, and we therefore know\nthat those padding bytes won't be read on the receiving end.\n\nWe could simply suppress this warning under MSAN with the approporiate\nifdef'd attributes, but a less intrusive solution is to 0-initialise the\nstruct, which guarantees that the padding will also be initialised.\n\nInterestingly, in the error-case branch, we only try to copy the first\ntwo members of pc_item_result, by copying only PC_ITEM_RESULT_BASE_SIZE\nbytes. However PC_ITEM_RESULT_BASE_SIZE is defined as\n'offsetof(the_last_member)', which means that we're copying padding bytes\nafter the end of the second last member. We could avoid doing this by\nredefining PC_ITEM_RESULT_BASE_SIZE as\n'offsetof(second_last_member) + sizeof(second_last_member)', but there's\nno huge benefit to doing so (and this patch silences the MSAN warning in\nthis scenario either way).\n\nMSAN output from t2080 (partially interleaved due to the\nparallel work :) ):\n\nUninitialized bytes in __interceptor_write at offset 12 inside [0x7fff37d83408, 160)\n==23279==WARNING: MemorySanitizer: use-of-uninitialized-value\nUninitialized bytes in __interceptor_write at offset 12 inside [0x7ffdb8a07ec8, 160)\n==23280==WARNING: MemorySanitizer: use-of-uninitialized-value\n #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8\n #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21\n #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6\n #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6\n #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2\n #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3\n #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2\n #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #12 0x7f8778114349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120\n\n Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result'\n #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite\nExiting\n #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8\n #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21\n #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6\n #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6\n #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2\n #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3\n #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2\n #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11\n #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3\n #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4\n #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19\n #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11\n #12 0x7f2749a0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)\n #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120\n\n Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result'\n #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55\n\nSUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite\n\nSigned-off-by: Andrzej Hunt \n---\n builtin/checkout--worker.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout--worker.c b/builtin/checkout--worker.c\nindex 289a9b8f89d0..fb9fd13b73c4 100644\n--- a/builtin/checkout--worker.c\n+++ b/builtin/checkout--worker.c\n@@ -53,7 +53,7 @@ static void packet_to_pc_item(const char *buffer, int len,\n \n static void report_result(struct parallel_checkout_item *pc_item)\n {\n-\tstruct pc_item_result res;\n+\tstruct pc_item_result res = { 0 };\n \tsize_t size;\n \n \tres.id = pc_item->id;\n-- \ngitgitgadget\n"},{"id":"427362","messageId":"pull.1033.v2.git.git.1623685877.gitgitgadget@gmail.com","threadId":"55900","inReplyTo":"pull.1033.git.git.1623343712.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Fix uninitialised reads found with MSAN","fromName":"Andrzej Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-14T15:51:13Z","receivedAt":"2021-06-14T15:52:20Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"V2 replaces an #if'd memset with some brace initialisation (patch 3/3) as\nper review comments.\n\nI've also removed an irrelevant \"technically\" from commit message 2/3, and\nfixed a typo in commit message 3/3.\n\nAndrzej Hunt (3):\n bulk-checkin: make buffer reuse more obvious and safer\n split-index: use oideq instead of memcmp to compare object_id's\n builtin/checkout--worker: zero-initialise struct to avoid MSAN\n complaints\n\n builtin/checkout--worker.c | 2 +-\n bulk-checkin.c | 3 +--\n split-index.c | 3 ++-\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\n\nbase-commit: 62a8d224e6203d9d3d2d1d63a01cf5647ec312c9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1033%2Fahunt%2Fmsan-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1033/ahunt/msan-v2\nPull-Request: https://github.com/git/git/pull/1033\n\nRange-diff vs v1:\n\n 1: 7659d4bf13c2 = 1: 7659d4bf13c2 bulk-checkin: make buffer reuse more obvious and safer\n 2: 14b0d5dd7fce ! 2: 6943eb511bee split-index: use oideq instead of memcmp to compare object_id's\n @@ Commit message\n include that field when calling memcmp on a subset of the cache_entry.\n Depending on which hashing algorithm is being used, only part of\n object_id.hash is actually being used, therefore including it in a\n - memcmp() is technically incorrect. Instead we choose to exclude the\n - object_id when calling memcmp(), and call oideq() separately.\n + memcmp() is incorrect. Instead we choose to exclude the object_id when\n + calling memcmp(), and call oideq() separately.\n \n This issue was found when running t1700-split-index with MSAN, see MSAN\n output below (on my machine, offset 76 corresponds to 4 bytes after the\n 3: cd1e1f6985c7 ! 3: 4bdc0b77f6f2 builtin/checkout--worker: memset struct to avoid MSAN complaints\n @@ Metadata\n Author: Andrzej Hunt \n \n ## Commit message ##\n - builtin/checkout--worker: memset struct to avoid MSAN complaints\n + builtin/checkout--worker: zero-initialise struct to avoid MSAN complaints\n \n report_result() sends a struct to the parent process, but that struct\n - contains unintialised padding bytes. Running this code under MSAN\n - rightly triggers a warning - but we also don't care about this warning\n - because we control the receiving code, and we therefore know that those\n - padding bytes won't be read on the receiving end. Therefore we add a\n - memset to convince MSAN that this memory is safe to read - but only\n - when building with MSAN to avoid this cost in normal usage.\n + would contain uninitialised padding bytes. Running this code under MSAN\n + rightly triggers a warning - but we don't particularly care about this\n + warning because we control the receiving code, and we therefore know\n + that those padding bytes won't be read on the receiving end.\n +\n + We could simply suppress this warning under MSAN with the approporiate\n + ifdef'd attributes, but a less intrusive solution is to 0-initialise the\n + struct, which guarantees that the padding will also be initialised.\n \n Interestingly, in the error-case branch, we only try to copy the first\n two members of pc_item_result, by copying only PC_ITEM_RESULT_BASE_SIZE\n @@ Commit message\n after the end of the second last member. We could avoid doing this by\n redefining PC_ITEM_RESULT_BASE_SIZE as\n 'offsetof(second_last_member) + sizeof(second_last_member)', but there's\n - no huge benefit to doing so (and our memset hack silences the MSAN\n - warning in this scenario either way).\n + no huge benefit to doing so (and this patch silences the MSAN warning in\n + this scenario either way).\n \n MSAN output from t2080 (partially interleaved due to the\n parallel work :) ):\n @@ Commit message\n Signed-off-by: Andrzej Hunt \n \n ## builtin/checkout--worker.c ##\n -@@ builtin/checkout--worker.c: static void report_result(struct parallel_checkout_item *pc_item)\n - \tstruct pc_item_result res;\n +@@ builtin/checkout--worker.c: static void packet_to_pc_item(const char *buffer, int len,\n + \n + static void report_result(struct parallel_checkout_item *pc_item)\n + {\n +-\tstruct pc_item_result res;\n ++\tstruct pc_item_result res = { 0 };\n \tsize_t size;\n \n -+#if defined(__has_feature)\n -+# if __has_feature(memory_sanitizer)\n -+\t// MSAN workaround: res contains padding bytes, which will remain\n -+\t// permanently unintialised. Later, we read all of res in order to send\n -+\t// it to the parent process - and MSAN (rightly) complains that we're\n -+\t// reading those unintialised padding bytes. By memset'ing res we\n -+\t// guarantee that there are no uninitialised bytes.\n -+\tmemset(&res, 0, sizeof(res));\n -+#endif\n -+#endif\n -+\n \tres.id = pc_item->id;\n - \tres.status = pc_item->status;\n - \n\n-- \ngitgitgadget\n"},{"id":"427679","messageId":"1966d581-5a02-bd28-0700-c1eb4c833d1e@iee.email","threadId":"55900","inReplyTo":"pull.1033.git.git.1623343712.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Fix uninitialised reads found with MSAN","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-06-17T09:28:40Z","receivedAt":"2021-06-17T09:28:44Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 10/06/2021 17:48, Andrzej Hunt via GitGitGadget wrote:\n> This can be worked around with an annotation in zlib (which I'm trying to\n> submit upstream at [1]) \n> Andrzej\n>\n> [1] https://github.com/madler/zlib/pull/561\nAndrzey,\n\nJust had a look at the zlib PR and it has CI check failure asking for\nextra info.\n\nPhilip\n"},{"id":"427988","messageId":"ef984f75-f5f2-6945-597b-1bfcb8ffb169@ahunt.org","threadId":"55900","inReplyTo":"1966d581-5a02-bd28-0700-c1eb4c833d1e@iee.email","subject":"Re: [PATCH 0/3] Fix uninitialised reads found with MSAN","fromName":"Andrzej Hunt","fromEmail":"andrzej@ahunt.org","sentAt":"2021-06-20T15:19:40Z","receivedAt":"2021-06-20T15:23:10Z","isPatch":true,"sender":{"key":"andrzej@ahunt.org","avatar":"https://avatars.githubusercontent.com/u/1546915?v=4"},"body":"\n\nOn 17/06/2021 11:28, Philip Oakley wrote:\n> On 10/06/2021 17:48, Andrzej Hunt via GitGitGadget wrote:\n>> This can be worked around with an annotation in zlib (which I'm trying to\n>> submit upstream at [1])\n>> Andrzej\n>>\n>> [1] https://github.com/madler/zlib/pull/561\n> Andrzey,\n> \n> Just had a look at the zlib PR and it has CI check failure asking for\n> extra info.\n\nThanks for spotting this. This seems to be a configuration issue with \nzlib's CI itself, which I noticed is also affecting other PR's on the \nsame repo: /. I'll wait for feedback from the zlib maintainer for now!\n\nATB,\n\n\nAndrzej\n"}]}