fix: disallow removing extmarks in on_lines callbacks (#23219)

fix(extmarks): disallow removing extmarks in on_lines callbacks

decor_redraw_start (which runs before decor_providers_invoke_lines) gets
references for the extmarks on a specific line. If these extmarks are
deleted in on_lines callbacks then this results in a heap-use-after-free
error.

Fixes #22801

src/nvim/api/deprecated.c

@@ -172,7 +172,7 @@ Integer nvim_buf_set_virtual_text(Buffer buffer, Integer src_id, Integer line, A
   decor.priority = 0;
 
   extmark_set(buf, ns_id, NULL, (int)line, 0, -1, -1, &decor, true,
-              false, kExtmarkNoUndo);
+              false, kExtmarkNoUndo, NULL);
   return src_id;
 }
 

src/nvim/api/extmark.c

@@ -874,7 +874,10 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id, Integer line, Integer
 
     extmark_set(buf, (uint32_t)ns_id, &id, (int)line, (colnr_T)col, line2, col2,
                 has_decor ? &decor : NULL, right_gravity, end_right_gravity,
-                kExtmarkNoUndo);
+                kExtmarkNoUndo, err);
+    if (ERROR_SET(err)) {
+      goto error;
+    }
   }
 
   return (Integer)id;
@@ -903,6 +906,11 @@ Boolean nvim_buf_del_extmark(Buffer buffer, Integer ns_id, Integer id, Error *er
     return false;
   });
 
+  if (decor_state.running_on_lines) {
+    api_set_error(err, kErrorTypeValidation, "Cannot remove extmarks during on_line callbacks");
+    return false;
+  }
+
   return extmark_del(buf, (uint32_t)ns_id, (uint32_t)id);
 }
 
@@ -993,7 +1001,7 @@ Integer nvim_buf_add_highlight(Buffer buffer, Integer ns_id, String hl_group, In
   extmark_set(buf, ns, NULL,
               (int)line, (colnr_T)col_start,
               end_line, (colnr_T)col_end,
-              &decor, true, false, kExtmarkNoUndo);
+              &decor, true, false, kExtmarkNoUndo, NULL);
   return ns_id;
 }
 
@@ -1022,6 +1030,11 @@ void nvim_buf_clear_namespace(Buffer buffer, Integer ns_id, Integer line_start,
     return;
   });
 
+  if (decor_state.running_on_lines) {
+    api_set_error(err, kErrorTypeValidation, "Cannot remove extmarks during on_line callbacks");
+    return;
+  }
+
   if (line_end < 0 || line_end > MAXLNUM) {
     line_end = MAXLNUM;
   }
@@ -1051,11 +1064,13 @@ void nvim_buf_clear_namespace(Buffer buffer, Integer ns_id, Integer line_start,
 /// for the extmarks set/modified inside the callback anyway.
 ///
 /// Note: doing anything other than setting extmarks is considered experimental.
-/// Doing things like changing options are not expliticly forbidden, but is
+/// Doing things like changing options are not explicitly forbidden, but is
 /// likely to have unexpected consequences (such as 100% CPU consumption).
 /// doing `vim.rpcnotify` should be OK, but `vim.rpcrequest` is quite dubious
 /// for the moment.
 ///
+/// Note: It is not allowed to remove or update extmarks in 'on_line' callbacks.
+///
 /// @param ns_id  Namespace id from |nvim_create_namespace()|
 /// @param opts  Table of callbacks:
 ///             - on_start: called first on each screen redraw

src/nvim/decoration.c

@@ -65,7 +65,7 @@ void bufhl_add_hl_pos_offset(buf_T *buf, int src_id, int hl_id, lpos_T pos_start
     }
     extmark_set(buf, (uint32_t)src_id, NULL,
                 (int)lnum - 1, hl_start, (int)lnum - 1 + end_off, hl_end,
-                &decor, true, false, kExtmarkNoUndo);
+                &decor, true, false, kExtmarkNoUndo, NULL);
   }
 }
 

src/nvim/decoration.h

@@ -100,6 +100,11 @@ typedef struct {
   int conceal_attr;
 
   TriState spell;
+
+  // This is used to prevent removing/updating extmarks inside
+  // on_lines callbacks which is not allowed since it can lead to
+  // heap-use-after-free errors.
+  bool running_on_lines;
 } DecorState;
 
 EXTERN DecorState decor_state INIT(= { 0 });

src/nvim/decoration_provider.c

@@ -150,6 +150,7 @@ void decor_providers_invoke_win(win_T *wp, DecorProviders *providers,
 void decor_providers_invoke_line(win_T *wp, DecorProviders *providers, int row, bool *has_decor,
                                  char **err)
 {
+  decor_state.running_on_lines = true;
   for (size_t k = 0; k < kv_size(*providers); k++) {
     DecorProvider *p = kv_A(*providers, k);
     if (p && p->redraw_line != LUA_NOREF) {
@@ -167,6 +168,7 @@ void decor_providers_invoke_line(win_T *wp, DecorProviders *providers, int row,
       hl_check_ns();
     }
   }
+  decor_state.running_on_lines = false;
 }
 
 /// For each provider invoke the 'buf' callback for a given buffer.
@@ -179,8 +181,9 @@ void decor_providers_invoke_buf(buf_T *buf, DecorProviders *providers, char **er
   for (size_t i = 0; i < kv_size(*providers); i++) {
     DecorProvider *p = kv_A(*providers, i);
     if (p && p->redraw_buf != LUA_NOREF) {
-      MAXSIZE_TEMP_ARRAY(args, 1);
+      MAXSIZE_TEMP_ARRAY(args, 2);
       ADD_C(args, BUFFER_OBJ(buf->handle));
+      ADD_C(args, INTEGER_OBJ((int64_t)display_tick));
       decor_provider_invoke(p->ns_id, "buf", p->redraw_buf, args, true, err);
     }
   }

src/nvim/extmark.c

@@ -31,6 +31,7 @@
 #include <assert.h>
 #include <sys/types.h>
 
+#include "nvim/api/private/helpers.h"
 #include "nvim/buffer.h"
 #include "nvim/buffer_defs.h"
 #include "nvim/buffer_updates.h"
@@ -59,7 +60,7 @@ static uint32_t *buf_ns_ref(buf_T *buf, uint32_t ns_id, bool put)
 /// must not be used during iteration!
 void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col, int end_row,
                  colnr_T end_col, Decoration *decor, bool right_gravity, bool end_right_gravity,
-                 ExtmarkOp op)
+                 ExtmarkOp op, Error *err)
 {
   uint32_t *ns = buf_ns_ref(buf, ns_id, true);
   uint32_t id = idp ? *idp : 0;
@@ -88,6 +89,13 @@ void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col
     MarkTreeIter itr[1] = { 0 };
     mtkey_t old_mark = marktree_lookup_ns(buf->b_marktree, ns_id, id, false, itr);
     if (old_mark.id) {
+      if (decor_state.running_on_lines) {
+        if (err) {
+          api_set_error(err, kErrorTypeException,
+                        "Cannot change extmarks during on_line callbacks");
+        }
+        goto error;
+      }
       if (mt_paired(old_mark) || end_row > -1) {
         extmark_del(buf, ns_id, id);
       } else {
@@ -162,6 +170,13 @@ revised:
   if (idp) {
     *idp = id;
   }
+
+  return;
+
+error:
+  if (decor_full) {
+    decor_free(decor);
+  }
 }
 
 static bool extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col)