Previously, when promisor_remote_move_to_tail() is called for a
promisor_remote which is currently the *only* element in promisors, a
cycle is created in the promisors linked list. This cycle leads to a
double free later on in promisor_remote_clear(): promisors is set to
promisors->next (a no-op, as promisors->next == promisors); the previous
value of promisors is free()'d; then the new value of promisors (which
is equal to the previous value of promisors) is also free()'d. This
double-free error was unrecoverable for the user without removing the
filter or re-cloning the repo and hoping to miss this edge case.
Now, when promisor_remote_move_to_tail() would be a no-op, just do a
no-op. In cases of promisor_remote_move_to_tail() where n>1, it works
correctly.
Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
---
This change showed up for us in a user bugreport; I'm actually fairly
unfamiliar with the codebase here but given the drastic nature of the
failure, I wanted to get a fix up quickly. I'm still working on how to
reproduce this exact case in the test suite (and actually would
appreciate any pointers). Specifically, it looks like we only really
break if we have a single promisor_remote in the linked list, call
move_to_tail() on it at least once, and then call clear() on it without
adding another promisor_remote first.
promisor-remote.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/promisor-remote.c b/promisor-remote.c
index 9bc296cdde..dccd697c2d 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -89,6 +89,9 @@ static struct promisor_remote *promisor_remote_lookup(const char *remote_name,
static void promisor_remote_move_to_tail(struct promisor_remote *r,
struct promisor_remote *previous)
{
+ if (promisors == r && promisors->next == NULL)
+ return;
+
if (previous)
previous->next = r->next;
else
--
2.23.0.351.gc4317032e6-goog
