From 58d4d7f1c5a665111f05c61901a11a555703fe11 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 7 Jan 2022 11:55:47 +0100 Subject: [PATCH 01/14] fetch: fix deadlock when cleaning up lockfiles in async signals When fetching packfiles, we write a bunch of lockfiles for the packfiles we're writing into the repository. In order to not leave behind any cruft in case we exit or receive a signal, we register both an exit handler as well as signal handlers for common signals like SIGINT. These handlers will then unlink the locks and free the data structure tracking them. We have observed a deadlock in this logic though: (gdb) bt #0 __lll_lock_wait_private () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95 #1 0x00007f4932bea2cd in _int_free (av=0x7f4932f2eb20 , p=0x3e3e4200, have_lock=0) at malloc.c:3969 #2 0x00007f4932bee58c in __GI___libc_free (mem=) at malloc.c:2975 #3 0x0000000000662ab1 in string_list_clear () #4 0x000000000044f5bc in unlock_pack_on_signal () #5 #6 _int_free (av=0x7f4932f2eb20 , p=, have_lock=0) at malloc.c:4024 #7 0x00007f4932bee58c in __GI___libc_free (mem=) at malloc.c:2975 #8 0x000000000065afd5 in strbuf_release () #9 0x000000000066ddb9 in delete_tempfile () #10 0x0000000000610d0b in files_transaction_cleanup.isra () #11 0x0000000000611718 in files_transaction_abort () #12 0x000000000060d2ef in ref_transaction_abort () #13 0x000000000060d441 in ref_transaction_prepare () #14 0x000000000060e0b5 in ref_transaction_commit () #15 0x00000000004511c2 in fetch_and_consume_refs () #16 0x000000000045279a in cmd_fetch () #17 0x0000000000407c48 in handle_builtin () #18 0x0000000000408df2 in cmd_main () #19 0x00000000004078b5 in main () The process was killed with a signal, which caused the signal handler to kick in and try free the data structures after we have unlinked the locks. It then deadlocks while calling free(3P). The root cause of this is that it is not allowed to call certain functions in async-signal handlers, as specified by signal-safety(7). Next to most I/O functions, this list of disallowed functions also includes memory-handling functions like malloc(3P) and free(3P) because they may not be reentrant. As a result, if we execute such functions in the signal handler, then they may operate on inconistent state and fail in unexpected ways. Fix this bug by not calling non-async-signal-safe functions when running in the signal handler. We're about to re-raise the signal anyway and will thus exit, so it's not much of a problem to keep the string list of lockfiles untouched. Note that it's fine though to call unlink(2), so we'll still clean up the lockfiles correctly. Signed-off-by: Patrick Steinhardt Reviewed-by: brian m. carlson Signed-off-by: Junio C Hamano --- builtin/clone.c | 2 +- builtin/fetch.c | 17 +++++++++++------ transport.c | 11 ++++++++--- transport.h | 14 +++++++++++++- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index fb377b2765..dd42c89b68 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1290,7 +1290,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) */ submodule_progress = transport->progress; - transport_unlock_pack(transport); + transport_unlock_pack(transport, 0); transport_disconnect(transport); if (option_dissociate) { diff --git a/builtin/fetch.c b/builtin/fetch.c index f7abbc31ff..7edf98e33b 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -222,17 +222,22 @@ static struct option builtin_fetch_options[] = { OPT_END() }; -static void unlock_pack(void) +static void unlock_pack(unsigned int flags) { if (gtransport) - transport_unlock_pack(gtransport); + transport_unlock_pack(gtransport, flags); if (gsecondary) - transport_unlock_pack(gsecondary); + transport_unlock_pack(gsecondary, flags); +} + +static void unlock_pack_atexit(void) +{ + unlock_pack(0); } static void unlock_pack_on_signal(int signo) { - unlock_pack(); + unlock_pack(TRANSPORT_UNLOCK_PACK_IN_SIGNAL_HANDLER); sigchain_pop(signo); raise(signo); } @@ -1326,7 +1331,7 @@ static int fetch_and_consume_refs(struct transport *transport, struct ref *ref_m trace2_region_leave("fetch", "consume_refs", the_repository); out: - transport_unlock_pack(transport); + transport_unlock_pack(transport, 0); return ret; } @@ -1962,7 +1967,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv, gtransport->server_options = &server_options; sigchain_push_common(unlock_pack_on_signal); - atexit(unlock_pack); + atexit(unlock_pack_atexit); sigchain_push(SIGPIPE, SIG_IGN); exit_code = do_fetch(gtransport, &rs); sigchain_pop(SIGPIPE); diff --git a/transport.c b/transport.c index e4f1decae2..3030ce81a6 100644 --- a/transport.c +++ b/transport.c @@ -1457,13 +1457,18 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs) return rc; } -void transport_unlock_pack(struct transport *transport) +void transport_unlock_pack(struct transport *transport, unsigned int flags) { + int in_signal_handler = !!(flags & TRANSPORT_UNLOCK_PACK_IN_SIGNAL_HANDLER); int i; for (i = 0; i < transport->pack_lockfiles.nr; i++) - unlink_or_warn(transport->pack_lockfiles.items[i].string); - string_list_clear(&transport->pack_lockfiles, 0); + if (in_signal_handler) + unlink(transport->pack_lockfiles.items[i].string); + else + unlink_or_warn(transport->pack_lockfiles.items[i].string); + if (!in_signal_handler) + string_list_clear(&transport->pack_lockfiles, 0); } int transport_connect(struct transport *transport, const char *name, diff --git a/transport.h b/transport.h index 8bb4c8bbc8..3f16e50c19 100644 --- a/transport.h +++ b/transport.h @@ -279,7 +279,19 @@ const struct ref *transport_get_remote_refs(struct transport *transport, */ const struct git_hash_algo *transport_get_hash_algo(struct transport *transport); int transport_fetch_refs(struct transport *transport, struct ref *refs); -void transport_unlock_pack(struct transport *transport); + +/* + * If this flag is set, unlocking will avoid to call non-async-signal-safe + * functions. This will necessarily leave behind some data structures which + * cannot be cleaned up. + */ +#define TRANSPORT_UNLOCK_PACK_IN_SIGNAL_HANDLER (1 << 0) + +/* + * Unlock all packfiles locked by the transport. + */ +void transport_unlock_pack(struct transport *transport, unsigned int flags); + int transport_disconnect(struct transport *transport); char *transport_anonymize_url(const char *url); void transport_take_over(struct transport *transport, From 4a9b204920152c668228a9d43a63be39b0c32f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Sat, 8 Jan 2022 16:02:30 +0000 Subject: [PATCH 02/14] lazyload: use correct calling conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Christoph Reiter reported on the Git for Windows issue tracker[1], that mingw_strftime() imports strftime() from ucrtbase.dll with the wrong calling convention. It should be __cdecl instead of WINAPI, which we always use in DECLARE_PROC_ADDR(). The MSYS2 project encountered cmake sefaults on x86 Windows caused by the same issue in the cmake source. [2] There are no known git crashes that where caused by this, yet, but we should try to prevent them. We import two other non-WINAPI functions via DECLARE_PROC_ADDR(), too. * NtSetSystemInformation() (NTAPI) * GetUserNameExW() (SEC_ENTRY) NTAPI, SEC_ENTRY and WINAPI are all ususally defined as __stdcall, but there are circumstances where they're defined differently. Teach DECLARE_PROC_ADDR() about calling conventions and be explicit about when we want to use which calling convention. Import winnt.h for the definition of NTAPI and sspi.h for SEC_ENTRY near their respective only users. [1] https://github.com/git-for-windows/git/issues/3560 [2] https://github.com/msys2/MINGW-packages/issues/10152 Reported-By: Christoph Reiter Signed-off-by: Matthias Aßhauer Signed-off-by: Junio C Hamano --- compat/mingw.c | 6 ++++-- compat/win32/lazyload.h | 6 +++--- compat/win32/trace2_win32_process_info.c | 4 ++-- compat/winansi.c | 5 +++-- t/helper/test-drop-caches.c | 4 +++- 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 9e0cd1e097..f38c438112 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -8,6 +8,8 @@ #include "win32/lazyload.h" #include "../config.h" #include "dir.h" +#define SECURITY_WIN32 +#include #define HCAST(type, handle) ((type)(intptr_t)handle) @@ -1008,7 +1010,7 @@ size_t mingw_strftime(char *s, size_t max, /* a pointer to the original strftime in case we can't find the UCRT version */ static size_t (*fallback)(char *, size_t, const char *, const struct tm *) = strftime; size_t ret; - DECLARE_PROC_ADDR(ucrtbase.dll, size_t, strftime, char *, size_t, + DECLARE_PROC_ADDR(ucrtbase.dll, size_t, __cdecl, strftime, char *, size_t, const char *, const struct tm *); if (INIT_PROC_ADDR(strftime)) @@ -2183,7 +2185,7 @@ enum EXTENDED_NAME_FORMAT { static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type) { - DECLARE_PROC_ADDR(secur32.dll, BOOL, GetUserNameExW, + DECLARE_PROC_ADDR(secur32.dll, BOOL, SEC_ENTRY, GetUserNameExW, enum EXTENDED_NAME_FORMAT, LPCWSTR, PULONG); static wchar_t wbuffer[1024]; DWORD len; diff --git a/compat/win32/lazyload.h b/compat/win32/lazyload.h index 2b3637135f..f2bb96c89c 100644 --- a/compat/win32/lazyload.h +++ b/compat/win32/lazyload.h @@ -4,7 +4,7 @@ /* * A pair of macros to simplify loading of DLL functions. Example: * - * DECLARE_PROC_ADDR(kernel32.dll, BOOL, CreateHardLinkW, + * DECLARE_PROC_ADDR(kernel32.dll, BOOL, WINAPI, CreateHardLinkW, * LPCWSTR, LPCWSTR, LPSECURITY_ATTRIBUTES); * * if (!INIT_PROC_ADDR(CreateHardLinkW)) @@ -25,10 +25,10 @@ struct proc_addr { }; /* Declares a function to be loaded dynamically from a DLL. */ -#define DECLARE_PROC_ADDR(dll, rettype, function, ...) \ +#define DECLARE_PROC_ADDR(dll, rettype, convention, function, ...) \ static struct proc_addr proc_addr_##function = \ { #dll, #function, NULL, 0 }; \ - typedef rettype (WINAPI *proc_type_##function)(__VA_ARGS__); \ + typedef rettype (convention *proc_type_##function)(__VA_ARGS__); \ static proc_type_##function function /* diff --git a/compat/win32/trace2_win32_process_info.c b/compat/win32/trace2_win32_process_info.c index 8ccbd1c2c6..a53fd92434 100644 --- a/compat/win32/trace2_win32_process_info.c +++ b/compat/win32/trace2_win32_process_info.c @@ -143,8 +143,8 @@ static void get_is_being_debugged(void) */ static void get_peak_memory_info(void) { - DECLARE_PROC_ADDR(psapi.dll, BOOL, GetProcessMemoryInfo, HANDLE, - PPROCESS_MEMORY_COUNTERS, DWORD); + DECLARE_PROC_ADDR(psapi.dll, BOOL, WINAPI, GetProcessMemoryInfo, + HANDLE, PPROCESS_MEMORY_COUNTERS, DWORD); if (INIT_PROC_ADDR(GetProcessMemoryInfo)) { PROCESS_MEMORY_COUNTERS pmc; diff --git a/compat/winansi.c b/compat/winansi.c index c27b20a79d..4fceecf14c 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -45,8 +45,9 @@ typedef struct _CONSOLE_FONT_INFOEX { static void warn_if_raster_font(void) { DWORD fontFamily = 0; - DECLARE_PROC_ADDR(kernel32.dll, BOOL, GetCurrentConsoleFontEx, - HANDLE, BOOL, PCONSOLE_FONT_INFOEX); + DECLARE_PROC_ADDR(kernel32.dll, BOOL, WINAPI, + GetCurrentConsoleFontEx, HANDLE, BOOL, + PCONSOLE_FONT_INFOEX); /* don't bother if output was ascii only */ if (!non_ascii_used) diff --git a/t/helper/test-drop-caches.c b/t/helper/test-drop-caches.c index 7b4278462b..e37396dd9c 100644 --- a/t/helper/test-drop-caches.c +++ b/t/helper/test-drop-caches.c @@ -3,6 +3,7 @@ #if defined(GIT_WINDOWS_NATIVE) #include "lazyload.h" +#include static int cmd_sync(void) { @@ -86,7 +87,8 @@ static int cmd_dropcaches(void) { HANDLE hProcess = GetCurrentProcess(); HANDLE hToken; - DECLARE_PROC_ADDR(ntdll.dll, DWORD, NtSetSystemInformation, INT, PVOID, ULONG); + DECLARE_PROC_ADDR(ntdll.dll, DWORD, NTAPI, NtSetSystemInformation, INT, PVOID, + ULONG); SYSTEM_MEMORY_LIST_COMMAND command; int status; From 97d6fb5a1f5c854d87fd3fd98722c469a9195a46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Mon, 10 Jan 2022 19:41:34 +0100 Subject: [PATCH 03/14] cache.h: drop duplicate `ensure_full_index()` declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are two identical declarations of `ensure_full_index()` in cache.h. Commit 3964fc2aae ("sparse-index: add guard to ensure full index", 2021-03-30) provided an empty implementation of `ensure_full_index()`, declaring it in a new file sparse-index.h. When commit 4300f8442a ("sparse-index: implement ensure_full_index()", 2021-03-30) fleshed out the implementation, it added an identical declaration to cache.h. Then 118a2e8bde ("cache: move ensure_full_index() to cache.h", 2021-04-01) favored having the declaration in cache.h. Because of the double declaration, at that point we could have just dropped the one in sparse-index.h, but instead it got moved to cache.h. As a result, cache.h contains the exact same function declaration twice. Drop the one under "/* Name hashing */", in favor of the one under "/* Initialize and use the cache information */". Signed-off-by: Martin Ågren Acked-by: Victoria Dye Signed-off-by: Junio C Hamano --- cache.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/cache.h b/cache.h index eba12487b9..c7543a1fce 100644 --- a/cache.h +++ b/cache.h @@ -350,8 +350,6 @@ void add_name_hash(struct index_state *istate, struct cache_entry *ce); void remove_name_hash(struct index_state *istate, struct cache_entry *ce); void free_name_hash(struct index_state *istate); -void ensure_full_index(struct index_state *istate); - /* Cache entry creation and cleanup */ /* From c39fc06b999305963600358f3f5e99698440cad2 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 10 Jan 2022 16:19:06 -0500 Subject: [PATCH 04/14] fmt-merge-msg: prevent use-after-free with signed tags When merging a signed tag, fmt_merge_msg_sigs() is responsible for populating the body of the merge message with the names of the signed tags, their signatures, and the validity of those signatures. In 02769437e1 (ssh signing: use sigc struct to pass payload, 2021-12-09), check_signature() was taught to pass the object payload via the sigc struct instead of passing the payload buffer separately. In effect, 02769437e1 causes buf, and sigc.payload to point at the same region in memory. This causes a problem for fmt_tag_signature(), which wants to read from this location, since it is freed beforehand by signature_check_clear() (which frees it via sigc's `payload` member). That makes the subsequent use in fmt_tag_signature() a use-after-free. As a result, merge messages did not contain the body of any signed tags. Luckily, they tend not to contain garbage, either, since the result of strstr()-ing the object buffer in fmt_tag_signature() is guarded: const char *tag_body = strstr(buf, "\n\n"); if (tag_body) { tag_body += 2; strbuf_add(tagbuf, tag_body, buf + len - tag_body); } Unfortunately, the tests in t6200 did not catch this at the time because they do not search for the body of signed tags in fmt-merge-msg's output. Resolve this by waiting to call signature_check_clear() until after its contents can be safely discarded. Harden ourselves against any future regressions in this area by making sure we can find signed tag messages in the output of fmt-merge-msg, too. Reported-by: Linus Torvalds Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- fmt-merge-msg.c | 2 +- t/t6200-fmt-merge-msg.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index e4f7810be2..422bd0c055 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -541,7 +541,6 @@ static void fmt_merge_msg_sigs(struct strbuf *out) else strbuf_addstr(&sig, sigc.output); } - signature_check_clear(&sigc); if (!tag_number++) { fmt_tag_signature(&tagbuf, &sig, buf, len); @@ -565,6 +564,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) } strbuf_release(&payload); strbuf_release(&sig); + signature_check_clear(&sigc); next: free(origbuf); } diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 6e10a539ce..d3e847f5e2 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -126,6 +126,7 @@ test_expect_success GPG 'message for merging local tag signed by good key' ' git fetch . signed-good-tag && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-tag${apos}" actual && + grep "^signed-tag-msg" actual && grep "^# gpg: Signature made" actual && grep "^# gpg: Good signature from" actual ' @@ -135,6 +136,7 @@ test_expect_success GPG 'message for merging local tag signed by unknown key' ' git fetch . signed-good-tag && GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-tag${apos}" actual && + grep "^signed-tag-msg" actual && grep "^# gpg: Signature made" actual && grep -E "^# gpg: Can${apos}t check signature: (public key not found|No public key)" actual ' @@ -145,6 +147,7 @@ test_expect_success GPGSSH 'message for merging local tag signed by good ssh key git fetch . signed-good-ssh-tag && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-ssh-tag${apos}" actual && + grep "^signed-ssh-tag-msg" actual && grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual ' @@ -155,6 +158,7 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh git fetch . signed-untrusted-ssh-tag && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-untrusted-ssh-tag${apos}" actual && + grep "^signed-ssh-tag-msg-untrusted" actual && grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual && grep "${GPGSSH_KEY_NOT_TRUSTED}" actual @@ -166,6 +170,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . expired-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}expired-signed${apos}" actual && + grep "^expired-signed" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' @@ -175,6 +180,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . notyetvalid-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}notyetvalid-signed${apos}" actual && + grep "^notyetvalid-signed" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' @@ -184,6 +190,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . timeboxedvalid-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}timeboxedvalid-signed${apos}" actual && + grep "^timeboxedvalid-signed" actual && grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual ' @@ -194,6 +201,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign git fetch . timeboxedinvalid-signed && git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}timeboxedinvalid-signed${apos}" actual && + grep "^timeboxedinvalid-signed" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' From 68d924e1de83b1e865acdca22b17a20fdec3add6 Mon Sep 17 00:00:00 2001 From: Bagas Sanjaya Date: Tue, 11 Jan 2022 19:36:27 +0700 Subject: [PATCH 05/14] branch: missing space fix at line 313 The message introduced by commit 593a2a5d06 (branch: protect branches checked out in all worktrees, 2021-12-01) is missing a space in the first line, add it. Signed-off-by: Bagas Sanjaya Signed-off-by: Junio C Hamano --- branch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/branch.c b/branch.c index 2cfe496d24..c37d9fb56b 100644 --- a/branch.c +++ b/branch.c @@ -212,7 +212,7 @@ int validate_new_branchname(const char *name, struct strbuf *ref, int force) worktrees = get_worktrees(); wt = find_shared_symref(worktrees, "HEAD", ref->buf); if (wt && !wt->is_bare) - die(_("cannot force update the branch '%s'" + die(_("cannot force update the branch '%s' " "checked out at '%s'"), ref->buf + strlen("refs/heads/"), wt->path); free_worktrees(worktrees); From 0517f591ca290a14ee3e516e478e8d2b78b45822 Mon Sep 17 00:00:00 2001 From: Fabian Stelzer Date: Wed, 12 Jan 2022 13:07:57 +0100 Subject: [PATCH 06/14] t/gpg: simplify test for unknown key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To test for a key that is completely unknown to the keyring we need one to sign the commit with. This was done by generating a new key and not add it into the keyring. To avoid the key generation overhead and problems where GPG did hang in CI during it, switch GNUPGHOME to the empty $GNUPGHOME_NOT_USED instead, therefore making all used keys unknown for this single `verify-commit` call. Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Fabian Stelzer Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/t7510-signed-commit.sh | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index d65a0171f2..50721aaf79 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -71,25 +71,7 @@ test_expect_success GPG 'create signed commits' ' git tag eleventh-signed $(cat oid) && echo 12 | git commit-tree --gpg-sign=B7227189 HEAD^{tree} >oid && test_line_count = 1 oid && - git tag twelfth-signed-alt $(cat oid) && - - cat >keydetails <<-\EOF && - Key-Type: RSA - Key-Length: 2048 - Subkey-Type: RSA - Subkey-Length: 2048 - Name-Real: Unknown User - Name-Email: unknown@git.com - Expire-Date: 0 - %no-ask-passphrase - %no-protection - EOF - gpg --batch --gen-key keydetails && - echo 13 >file && git commit -a -S"unknown@git.com" -m thirteenth && - git tag thirteenth-signed && - DELETE_FINGERPRINT=$(gpg -K --with-colons --fingerprint --batch unknown@git.com | grep "^fpr" | head -n 1 | awk -F ":" "{print \$10;}") && - gpg --batch --yes --delete-secret-keys $DELETE_FINGERPRINT && - gpg --batch --yes --delete-keys unknown@git.com + git tag twelfth-signed-alt $(cat oid) ' test_expect_success GPG 'verify and show signatures' ' @@ -129,7 +111,7 @@ test_expect_success GPG 'verify and show signatures' ' ' test_expect_success GPG 'verify-commit exits failure on unknown signature' ' - test_must_fail git verify-commit thirteenth-signed 2>actual && + test_must_fail env GNUPGHOME="$GNUPGHOME_NOT_USED" git verify-commit initial 2>actual && ! grep "Good signature from" actual && ! grep "BAD signature from" actual && grep -q -F -e "No public key" -e "public key not found" actual From a5c97b016421a2869b460bbf6bdcd43dc186d433 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 12 Jan 2022 12:11:42 -0800 Subject: [PATCH 07/14] packfile: fix off-by-one error in decoding logic shift count being exactly at 7-bit smaller than the long is OK; on 32-bit architecture, shift count starts at 4 and goes through 11, 18 and 25, at which point the guard triggers one iteration too early. Reported-by: Marc Strapetz Signed-off-by: Junio C Hamano --- packfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packfile.c b/packfile.c index d3820c780b..667e21ce97 100644 --- a/packfile.c +++ b/packfile.c @@ -1067,7 +1067,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf, size = c & 15; shift = 4; while (c & 0x80) { - if (len <= used || (bitsizeof(long) - 7) <= shift) { + if (len <= used || (bitsizeof(long) - 7) < shift) { error("bad object header"); size = used = 0; break; From 68d1da41c4ecbc054ec38e11b30008dba32a7791 Mon Sep 17 00:00:00 2001 From: "Randall S. Becker" Date: Mon, 10 Jan 2022 22:51:39 -0500 Subject: [PATCH 08/14] build: NonStop ships with an older zlib Notably, it lacks uncompress2(); use the fallback we ship in our tree instead. Signed-off-by: Randall S. Becker Signed-off-by: Junio C Hamano --- config.mak.uname | 1 + 1 file changed, 1 insertion(+) diff --git a/config.mak.uname b/config.mak.uname index a3a779327f..9b3e9bff5f 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -576,6 +576,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL) NO_SETENV = YesPlease NO_UNSETENV = YesPlease NO_MKDTEMP = YesPlease + NO_UNCOMPRESS2 = YesPlease # Currently libiconv-1.9.1. OLD_ICONV = UnfortunatelyYes NO_REGEX = NeedsStartEnd From 1ffcbaa1a5f10c9f706314d77f88de20a4a498c2 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 12 Jan 2022 16:27:08 -0800 Subject: [PATCH 09/14] Last minute fixes before -rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.35.0.txt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.35.0.txt b/Documentation/RelNotes/2.35.0.txt index 26efaf1cbc..fa5a7318a8 100644 --- a/Documentation/RelNotes/2.35.0.txt +++ b/Documentation/RelNotes/2.35.0.txt @@ -100,7 +100,7 @@ Performance, Internal Implementation, Development Support etc. * Teach and encourage first-time contributors to this project to state the base commit when they submit their topic. - * The command line complation for "git send-email" options have been + * The command line completion for "git send-email" options have been tweaked to make it easier to keep it in sync with the command itself. * Ensure that the sparseness of the in-core index matches the @@ -367,6 +367,13 @@ Fixes since v2.34 it failed to restore changes to tracked ones. (merge 71cade5a0b en/stash-df-fix later to maint). + * Calling dynamically loaded functions on Windows has been corrected. + (merge 4a9b204920 ma/windows-dynload-fix later to maint). + + * Some lockfile code called free() in signal-death code path, which + has been corrected. + (merge 58d4d7f1c5 ps/lockfile-cleanup-fix later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 74db416c9c cw/protocol-v2-doc-fix later to maint). (merge f9b2b6684d ja/doc-cleanup later to maint). @@ -391,3 +398,5 @@ Fixes since v2.34 (merge 999bba3e0b rs/daemon-plug-leak later to maint). (merge 786eb1ba39 js/l10n-mention-ngettext-early-in-readme later to maint). (merge 2f12b31b74 ab/makefile-msgfmt-wo-stats later to maint). + (merge 0517f591ca fs/gpg-unknown-key-test-fix later to maint). + (merge 97d6fb5a1f ma/header-dup-cleanup later to maint). From cac15b3fb422efb2cd1572cc654793d5df5fa434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 12 Jan 2022 13:36:46 +0100 Subject: [PATCH 10/14] refs API: use "failure_errno", not "errno" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a logic error in refs_resolve_ref_unsafe() introduced in a recent series of mine to abstract the refs API away from errno. See 96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29)for that series. In that series introduction of "failure_errno" to refs_resolve_ref_unsafe came in ef18119dec8 (refs API: add a version of refs_resolve_ref_unsafe() with "errno", 2021-10-16). There we'd set "errno = 0" immediately before refs_read_raw_ref(), and then set "failure_errno" to "errno" if errno was non-zero afterwards. Then in the next commit 8b72fea7e91 (refs API: make refs_read_raw_ref() not set errno, 2021-10-16) we started expecting "refs_read_raw_ref()" to set "failure_errno". It would do that if refs_read_raw_ref() failed, but it wouldn't be the same errno. So we might set the "errno" here to any arbitrary bad value, and end up e.g. returning NULL when we meant to return the refname from refs_resolve_ref_unsafe(), or the other way around. Instrumenting this code will reveal cases where refs_read_raw_ref() will fail, and "errno" and "failure_errno" will be set to different values. In practice I haven't found a case where this scary bug changed anything in practice. The reason for that is that we'll not care about the actual value of "errno" here per-se, but only whether: 1. We have an errno 2. If it's one of ENOENT, EISDIR or ENOTDIR. See the adjacent code added in a1c1d8170db (refs_resolve_ref_unsafe: handle d/f conflicts for writes, 2017-10-06) I.e. if we clobber "failure_errno" with "errno", but it happened to be one of those three, and we'll clobber it with another one of the three we were OK. Perhaps there are cases where the difference ended up mattering, but I haven't found them. Instrumenting the test suite to fail if "errno" and "failure_errno" are different shows a lot of failures, checking if they're different *and* one is but not the other is outside that list of three "errno" values yields no failures. But let's fix the obvious bug. We should just stop paying attention to "errno" in refs_resolve_ref_unsafe(). In addition let's change the partial resetting of "errno" in files_read_raw_ref() to happen just before the "return", to ensure that any such bug will be more easily spotted in the future. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- refs.c | 2 -- refs/files-backend.c | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/refs.c b/refs.c index cd9b8bb114..fe373bae4e 100644 --- a/refs.c +++ b/refs.c @@ -1713,8 +1713,6 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs, if (refs_read_raw_ref(refs, refname, oid, &sb_refname, &read_flags, failure_errno)) { *flags |= read_flags; - if (errno) - *failure_errno = errno; /* In reading mode, refs must eventually resolve */ if (resolve_flags & RESOLVE_REF_READING) diff --git a/refs/files-backend.c b/refs/files-backend.c index abb6091fcf..4bb4de72b6 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -385,7 +385,6 @@ stat_ref: if (lstat(path, &st) < 0) { int ignore_errno; myerr = errno; - errno = 0; if (myerr != ENOENT) goto out; if (refs_read_raw_ref(refs->packed_ref_store, refname, oid, @@ -402,7 +401,6 @@ stat_ref: strbuf_reset(&sb_contents); if (strbuf_readlink(&sb_contents, path, st.st_size) < 0) { myerr = errno; - errno = 0; if (myerr == ENOENT || myerr == EINVAL) /* inconsistent with lstat; retry */ goto stat_ref; @@ -472,6 +470,7 @@ out: strbuf_release(&sb_path); strbuf_release(&sb_contents); + errno = 0; return ret; } From 59069107948bc87b8b6d46d49a52df410c4a8745 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Thu, 13 Jan 2022 21:28:45 +0100 Subject: [PATCH 11/14] t1450-fsck: exec-bit is not needed to make loose object writable A test case wants to append stuff to a loose object file to ensure that this kind of corruption is detected. To make a read-only loose object file writable with chmod, it is not necessary to also make it executable. Replace the bitmask 755 with the instruction +w to request only the write bit and to also heed the umask. And get rid of a POSIXPERM prerequisite, which is unnecessary for the test. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t1450-fsck.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 6337236fd8..de50c0ea01 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -94,13 +94,13 @@ test_expect_success 'object with hash and type mismatch' ' ) ' -test_expect_success POSIXPERM 'zlib corrupt loose object output ' ' +test_expect_success 'zlib corrupt loose object output ' ' git init --bare corrupt-loose-output && ( cd corrupt-loose-output && oid=$(git hash-object -w --stdin --literally >$oidf && cat >expect.error <<-EOF && From f2b255141b3008a46b4946e7da44b966797e4355 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Thu, 13 Jan 2022 16:55:34 +0000 Subject: [PATCH 12/14] reftable: avoid initializing structs from structs Apparently, the IBM xlc compiler doesn't like this. Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano --- reftable/merged_test.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/reftable/merged_test.c b/reftable/merged_test.c index 24461e8a80..abd34849fc 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -207,11 +207,11 @@ static void test_merged(void) }, }; - struct reftable_ref_record want[] = { - r2[0], - r1[1], - r3[0], - r3[1], + struct reftable_ref_record *want[] = { + &r2[0], + &r1[1], + &r3[0], + &r3[1], }; struct reftable_ref_record *refs[] = { r1, r2, r3 }; @@ -250,7 +250,7 @@ static void test_merged(void) EXPECT(ARRAY_SIZE(want) == len); for (i = 0; i < len; i++) { - EXPECT(reftable_ref_record_equal(&want[i], &out[i], + EXPECT(reftable_ref_record_equal(want[i], &out[i], GIT_SHA1_RAWSZ)); } for (i = 0; i < len; i++) { @@ -345,10 +345,10 @@ static void test_merged_logs(void) .value_type = REFTABLE_LOG_DELETION, }, }; - struct reftable_log_record want[] = { - r2[0], - r3[0], - r1[1], + struct reftable_log_record *want[] = { + &r2[0], + &r3[0], + &r1[1], }; struct reftable_log_record *logs[] = { r1, r2, r3 }; @@ -387,7 +387,7 @@ static void test_merged_logs(void) EXPECT(ARRAY_SIZE(want) == len); for (i = 0; i < len; i++) { - EXPECT(reftable_log_record_equal(&want[i], &out[i], + EXPECT(reftable_log_record_equal(want[i], &out[i], GIT_SHA1_RAWSZ)); } From 22d2f70e85e767abba2e284e32c0edb7f749e29c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 11 Jan 2022 17:40:23 +0100 Subject: [PATCH 13/14] reftable tests: avoid "int" overflow, use "uint64_t" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change code added in 1ae2b8cda84 (reftable: add merged table view, 2021-10-07) to consistently use the "uint64_t" type. These "min" and "max" variables get passed in the body of this function to a function whose prototype is: [...] reftable_writer_set_limits([...], uint64_t min, uint64_t max This avoids the following warning on SunCC 12.5 on gcc211.fsffrance.org: "reftable/merged_test.c", line 27: warning: initializer does not fit or is out of range: 0xffffffff Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- reftable/merged_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reftable/merged_test.c b/reftable/merged_test.c index abd34849fc..d08c16abef 100644 --- a/reftable/merged_test.c +++ b/reftable/merged_test.c @@ -24,8 +24,8 @@ https://developers.google.com/open-source/licenses/bsd static void write_test_table(struct strbuf *buf, struct reftable_ref_record refs[], int n) { - int min = 0xffffffff; - int max = 0; + uint64_t min = 0xffffffff; + uint64_t max = 0; int i = 0; int err; From df3c41adeb212432c53d93ce6ace5d5374dc6e11 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 14 Jan 2022 15:26:53 -0800 Subject: [PATCH 14/14] Git 2.35-rc1 Signed-off-by: Junio C Hamano --- GIT-VERSION-GEN | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 9a98b03aac..e9c1ad960f 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.35.0-rc0 +DEF_VER=v2.35.0-rc1 LF=' '