Make the pack and mwindow implementations data-race-free

This change fixes a packfile heap corruption that can happen when
interacting with multiple packfiles concurrently across multiple
threads. This is exacerbated by setting a lower mwindow open file limit.

This change:

* Renames most of the internal methods in pack.c to clearly indicate
  that they expect to be called with a certain lock held, making
  reasoning about the state of locks a bit easier.
* Splits the `git_pack_file` lock in two: the one in `git_pack_file`
  only protects the `index_map`. The protection to `git_mwindow_file` is
  now in that struct.
* Explicitly checks for freshness of the `git_pack_file` in
  `git_packfile_unpack_header`: this allows the mwindow implementation
  to close files whenever there is enough cache pressure, and
  `git_packfile_unpack_header` will reopen the packfile if needed.
* After a call to `p_munmap()`, the `data` and `len` fields are poisoned
  with `NULL` to make use-after-frees more evident and crash rather than
  being open to the possibility of heap corruption.
* Adds a test case to prevent this from regressing in the future.

Fixes: #5591

1 files changed, 13 insertions, 0 deletions

diff --git a/script/thread-sanitizer.supp b/script/thread-sanitizer.supp
index 359a9b35e..97d23046f 100644
--- a/script/thread-sanitizer.supp
+++ b/script/thread-sanitizer.supp
@@ -3,6 +3,19 @@
 # consistent lock hierarchy that is easy to understand.
 deadlock:attr_cache_lock
 
+# git_mwindow_file_register has the possibility of evicting some files from the
+# global cache. In order to avoid races and closing files that are currently
+# being accessed, before evicting any file it will attempt to acquire that
+# file's lock. Finally, git_mwindow_file_register is typically called with a
+# file lock held, because the caller will use the fd in the mwf immediately
+# after registering it. This causes ThreadSanitizer to observe different orders
+# of acquisition of the mutex (which implies a possibility of a deadlock),
+# _but_ since the files are added to the cache after other files have been
+# evicted, there cannot be a case where mwf A is trying to be registered while
+# evicting mwf B concurrently and viceversa: at most one of them can be present
+# in the cache.
+deadlock:git_mwindow_file_register
+
 # When invoking the time/timezone functions from git_signature_now(), they
 # access libc methods that need to be instrumented to correctly analyze the
 # data races.