From 0945be61c84427329010fc58b3eb11d4471a981e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Grubbstr=C3=B6m=20=28Grubba=29?=
<grubba@grubba.org>
Date: Mon, 22 Jan 2018 15:57:01 +0100
Subject: [PATCH] Backend: Fix race-condition in backend_do_call_outs().
There was a potential delay between a call_out running, and it
being removed from the call_hash hash table. This race caused
NULL-dereferences in backend_find_call_out(), and potentially
dereferences of pointers to freed memory on growing the hash.
Fixes some of [PIKE-55]/[PIKE-56]
---
src/backend.cmod | 41 +++++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/src/backend.cmod b/src/backend.cmod
index 88a7a63a3f..88f095c626 100644
--- a/src/backend.cmod
+++ b/src/backend.cmod
@@ -132,25 +132,15 @@ static int compat_box_dispatcher (struct fd_callback_box *box, int event);
/* CALL OUT STUFF */
-#ifdef PIKE_DEBUG
-#define MESS_UP_BLOCK(X) do {\
- (X)->next_arr=(struct Backend_CallOut_struct *)(ptrdiff_t)-1; \
- (X)->next_fun=(struct Backend_CallOut_struct *)(ptrdiff_t)-1; \
- (X)->prev_arr=(struct Backend_CallOut_struct **)(ptrdiff_t)-1; \
- (X)->prev_fun=(struct Backend_CallOut_struct **)(ptrdiff_t)-1; \
- (X)->pos=-1; \
- } while(0)
-#else
-#define MESS_UP_BLOCK(X)
-#endif
-
-#undef EXIT_BLOCK
-#define EXIT_BLOCK(X) do { \
- *(X->prev_arr)=X->next_arr; \
- if(X->next_arr) X->next_arr->prev_arr=X->prev_arr; \
- *(X->prev_fun)=X->next_fun; \
- if(X->next_fun) X->next_fun->prev_fun=X->prev_fun; \
- MESS_UP_BLOCK(X); \
+#define EXIT_CO(X) do { \
+ *(X->prev_arr) = X->next_arr; \
+ if(X->next_arr) X->next_arr->prev_arr = X->prev_arr; \
+ *(X->prev_fun) = X->next_fun; \
+ if(X->next_fun) X->next_fun->prev_fun = X->prev_fun; \
+ (X)->next_arr = NULL; \
+ (X)->next_fun = NULL; \
+ (X)->prev_arr = NULL; \
+ (X)->prev_fun = NULL; \
} while(0)
struct hash_ent
@@ -628,7 +618,7 @@ PIKECLASS Backend
free_object(this->this);
this->this = NULL;
}
- EXIT_BLOCK(this);
+ EXIT_CO(this);
}
/* Start a new call out */
@@ -924,6 +914,17 @@ PIKECLASS Backend
args = cc->args->size;
push_array_items(cc->args);
cc->args = NULL;
+ /* Unlink the call_out from the call_hash hash table directly.
+ *
+ * NB: Don't wait for the EXIT callback to run, as this
+ * would cause a race-condition with respect to
+ * - Growing the call_hash hash table (which would
+ * leave live pointers to the old hash table).
+ * - backend_find_call_out() (which would dereference
+ * cc->args (ie NULL)).
+ * until destruct_objects_to_destruct() is run.
+ */
+ EXIT_CO(cc);
free_object(cc->this);
check_destructed(Pike_sp - args);
--
GitLab