patch 9.0.0487: using freed memory with combination of closuresv9.0.0487

Problem:    Using freed memory with combination of closures.
Solution:   Do not use a partial after it has been freed through the
            funcstack.

4 files changed, 31 insertions, 19 deletions

diff --git a/src/eval.c b/src/eval.c
index f280e2afc6..3209d08dcc 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -4876,6 +4876,8 @@ partial_unref(partial_T *pt)
 {
     if (pt != NULL)
     {
+	int	done = FALSE;
+
 	if (--pt->pt_refcount <= 0)
 	    partial_free(pt);
 
@@ -4883,9 +4885,12 @@ partial_unref(partial_T *pt)
 	// only reference and can be freed if no other partials reference it.
 	else if (pt->pt_refcount == 1)
 	{
+	    // careful: if the funcstack is freed it may contain this partial
+	    // and it gets freed as well
 	    if (pt->pt_funcstack != NULL)
-		funcstack_check_refcount(pt->pt_funcstack);
-	    if (pt->pt_loopvars != NULL)
+		done = funcstack_check_refcount(pt->pt_funcstack);
+
+	    if (!done && pt->pt_loopvars != NULL)
 		loopvars_check_refcount(pt->pt_loopvars);
 	}
     }
diff --git a/src/proto/vim9execute.pro b/src/proto/vim9execute.pro
index a2b56c8bb8..708f69a0de 100644
--- a/src/proto/vim9execute.pro
+++ b/src/proto/vim9execute.pro
@@ -1,7 +1,7 @@
 /* vim9execute.c */
 void to_string_error(vartype_T vartype);
 void update_has_breakpoint(ufunc_T *ufunc);
-void funcstack_check_refcount(funcstack_T *funcstack);
+int funcstack_check_refcount(funcstack_T *funcstack);
 int set_ref_in_funcstacks(int copyID);
 int in_def_function(void);
 ectx_T *clear_currrent_ectx(void);
diff --git a/src/version.c b/src/version.c
index 2ce61d5747..a8d0f48157 100644
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    487,
+/**/
     486,
 /**/
     485,
diff --git a/src/vim9execute.c b/src/vim9execute.c
index 3b6a084d7b..1d30045df8 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -797,16 +797,19 @@ handle_closure_in_use(ectx_T *ectx, int free_arguments)
  * funcstack may be the only reference to the partials in the local variables.
  * Go over all of them, the funcref and can be freed if all partials
  * referencing the funcstack have a reference count of one.
+ * Returns TRUE if the funcstack is freed, the partial referencing it will then
+ * also have been freed.
  */
-    void
+    int
 funcstack_check_refcount(funcstack_T *funcstack)
 {
-    int		    i;
-    garray_T	    *gap = &funcstack->fs_ga;
-    int		    done = 0;
+    int		i;
+    garray_T	*gap = &funcstack->fs_ga;
+    int		done = 0;
+    typval_T	*stack;
 
     if (funcstack->fs_refcount > funcstack->fs_min_refcount)
-	return;
+	return FALSE;
     for (i = funcstack->fs_var_offset; i < gap->ga_len; ++i)
     {
 	typval_T *tv = ((typval_T *)gap->ga_data) + i;
@@ -816,18 +819,20 @@ funcstack_check_refcount(funcstack_T *funcstack)
 		&& tv->vval.v_partial->pt_refcount == 1)
 	    ++done;
     }
-    if (done == funcstack->fs_min_refcount)
-    {
-	typval_T	*stack = gap->ga_data;
+    if (done != funcstack->fs_min_refcount)
+	return FALSE;
 
-	// All partials referencing the funcstack have a reference count of
-	// one, thus the funcstack is no longer of use.
-	for (i = 0; i < gap->ga_len; ++i)
-	    clear_tv(stack + i);
-	vim_free(stack);
-	remove_funcstack_from_list(funcstack);
-	vim_free(funcstack);
-    }
+    stack = gap->ga_data;
+
+    // All partials referencing the funcstack have a reference count of
+    // one, thus the funcstack is no longer of use.
+    for (i = 0; i < gap->ga_len; ++i)
+	clear_tv(stack + i);
+    vim_free(stack);
+    remove_funcstack_from_list(funcstack);
+    vim_free(funcstack);
+
+    return TRUE;
 }
 
 /*