patch 9.0.0487: using freed memory with combination of closures

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

diff --git a/src/eval.c b/src/eval.c
index f280e2afc6906aec5e080411d401c0b071c269ca..3209d08dcca3f262d6dc8627a05e9c9c3f1152a7 100644 (file)
--- 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 a2b56c8bb8b6651b959d609169f0ac74a9fb8f93..708f69a0de7907d39cf44d06a01de8df56d5c4e8 100644 (file)
--- 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 2ce61d574779a12180beaa9402792ec838c891fa..a8d0f48157e0f90ce56cb218ece2d55d72dc10f1 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -703,6 +703,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    487,
 /**/
     486,
 /**/

diff --git a/src/vim9execute.c b/src/vim9execute.c
index 3b6a084d7b377346673d256d95a1c8f8d3475e1e..1d30045df8c4b21e22ca8cb6db94d0fbecc0dfc8 100644 (file)
--- 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;
 }
 
 /*