Merge pull request #37389 from zeertzjq/vim-9.1.2085

vim-patch:partial:9.0.0907,9.1.{1323,2023,2085}

src/nvim/buffer.c

@@ -129,6 +129,19 @@ typedef enum {
   kBffInitChangedtick = 2,
 } BufFreeFlags;
 
+static void trigger_undo_ftplugin(buf_T *buf, win_T *win)
+{
+  const bool win_was_locked = win->w_locked;
+  window_layout_lock();
+  buf->b_locked++;
+  win->w_locked = true;
+  // b:undo_ftplugin may be set, undo it
+  do_cmdline_cmd("if exists('b:undo_ftplugin') | exe b:undo_ftplugin | endif");
+  buf->b_locked--;
+  win->w_locked = win_was_locked;
+  window_layout_unlock();
+}
+
 /// Calculate the percentage that `part` is of the `whole`.
 int calc_percentage(int64_t part, int64_t whole)
 {
@@ -1945,6 +1958,7 @@ buf_T *buflist_new(char *ffname_arg, char *sfname_arg, linenr_T lnum, int flags)
     assert(curbuf != NULL);
     buf = curbuf;
     set_bufref(&bufref, buf);
+    trigger_undo_ftplugin(buf, curwin);
     // It's like this buffer is deleted.  Watch out for autocommands that
     // change curbuf!  If that happens, allocate a new buffer anyway.
     buf_freeall(buf, BFA_WIPE | BFA_DEL);

src/nvim/diff.c

@@ -855,6 +855,14 @@ static int diff_write(buf_T *buf, diffin_T *din, linenr_T start, linenr_T end)
     return diff_write_buffer(buf, &din->din_mmfile, start, end);
   }
 
+  // Writing the diff buffers may trigger changes in the window structure
+  // via aucmd_prepbuf()/aucmd_restbuf() commands.
+  // This may cause recursively calling winframe_remove() which is not safe and causes
+  // use after free, so let's stop it here.
+  if (frames_locked()) {
+    return FAIL;
+  }
+
   if (end < 0) {
     end = buf->b_ml.ml_line_count;
   }

src/nvim/errors.h

@@ -195,6 +195,8 @@ EXTERN const char e_stray_closing_curly_str[]
 INIT(= N_("E1278: Stray '}' without a matching '{': %s"));
 EXTERN const char e_missing_close_curly_str[]
 INIT(= N_("E1279: Missing '}': %s"));
+EXTERN const char e_not_allowed_to_change_window_layout_in_this_autocmd[]
+INIT(= N_("E1312: Not allowed to change the window layout in this autocmd"));
 
 EXTERN const char e_val_too_large[] INIT(= N_("E1510: Value too large: %s"));
 

src/nvim/ex_docmd.c

@@ -5033,6 +5033,9 @@ void ex_win_close(int forceit, win_T *win, tabpage_T *tp)
     emsg(_(e_autocmd_close));
     return;
   }
+  if (!win->w_floating && window_layout_locked()) {
+    return;
+  }
 
   buf_T *buf = win->w_buffer;
 
@@ -5074,6 +5077,10 @@ static void ex_tabclose(exarg_T *eap)
     return;
   }
 
+  if (window_layout_locked()) {
+    return;
+  }
+
   int tab_number = get_tabpage_arg(eap);
   if (eap->errmsg != NULL) {
     return;
@@ -5105,6 +5112,10 @@ static void ex_tabonly(exarg_T *eap)
     return;
   }
 
+  if (window_layout_locked()) {
+    return;
+  }
+
   int tab_number = get_tabpage_arg(eap);
   if (eap->errmsg != NULL) {
     return;
@@ -5175,9 +5186,12 @@ void tabpage_close_other(tabpage_T *tp, int forceit)
 /// ":only".
 static void ex_only(exarg_T *eap)
 {
-  win_T *wp;
+  if (window_layout_locked()) {
+    return;
+  }
 
   if (eap->addr_count > 0) {
+    win_T *wp;
     linenr_T wnr = eap->line2;
     for (wp = firstwin; --wnr > 0;) {
       if (wp->w_next == NULL) {
@@ -5185,11 +5199,9 @@ static void ex_only(exarg_T *eap)
       }
       wp = wp->w_next;
     }
-  } else {
-    wp = curwin;
-  }
-  if (wp != curwin) {
-    win_goto(wp);
+    if (wp != curwin) {
+      win_goto(wp);
+    }
   }
   close_others(true, eap->forceit);
 }
@@ -5201,11 +5213,11 @@ static void ex_hide(exarg_T *eap)
     return;
   }
 
+  win_T *win = NULL;
   if (eap->addr_count == 0) {
-    win_close(curwin, false, eap->forceit);  // don't free buffer
+    win = curwin;
   } else {
     int winnr = 0;
-    win_T *win = NULL;
 
     FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
       winnr++;
@@ -5217,8 +5229,13 @@ static void ex_hide(exarg_T *eap)
     if (win == NULL) {
       win = lastwin;
     }
-    win_close(win, false, eap->forceit);
   }
+
+  if (!win->w_floating && window_layout_locked()) {
+    return;
+  }
+
+  win_close(win, false, eap->forceit);  // don't free buffer
 }
 
 /// ":stop" and ":suspend": Suspend Vim.

src/nvim/window.c

@@ -108,6 +108,47 @@ static char *m_onlyone = N_("Already only one window");
 /// autocommands mess up the window structure.
 static int split_disallowed = 0;
 
+/// When non-zero closing a window is forbidden.  Used to avoid that nasty
+/// autocommands mess up the window structure.
+static int close_disallowed = 0;
+
+/// When non-zero changing the window frame structure is forbidden.  Used
+/// to avoid that winframe_remove() is called recursively
+static int frame_locked = 0;
+
+/// Disallow changing the window layout (split window, close window, move
+/// window).  Resizing is still allowed.
+/// Used for autocommands that temporarily use another window and need to
+/// make sure the previously selected window is still there.
+/// Must be matched with exactly one call to window_layout_unlock()!
+void window_layout_lock(void)
+{
+  split_disallowed++;
+  close_disallowed++;
+}
+
+void window_layout_unlock(void)
+{
+  split_disallowed--;
+  close_disallowed--;
+}
+
+bool frames_locked(void)
+{
+  return frame_locked;
+}
+
+/// When the window layout cannot be changed give an error and return true.
+bool window_layout_locked(void)
+{
+  // if (split_disallowed > 0 || close_disallowed > 0) {
+  if (close_disallowed > 0) {
+    emsg(_(e_not_allowed_to_change_window_layout_in_this_autocmd));
+    return true;
+  }
+  return false;
+}
+
 // #define WIN_DEBUG
 #ifdef WIN_DEBUG
 /// Call this method to log the current window layout.
@@ -2739,6 +2780,9 @@ int win_close(win_T *win, bool free_buf, bool force)
     emsg(_(e_cannot_close_last_window));
     return FAIL;
   }
+  if (!win->w_floating && window_layout_locked()) {
+    return FAIL;
+  }
 
   if (win_locked(win)
       || (win->w_buffer != NULL && win->w_buffer->b_locked > 0)) {
@@ -3271,6 +3315,8 @@ win_T *winframe_remove(win_T *win, int *dirp, tabpage_T *tp, frame_T **unflat_al
 
   frame_T *frp_close = win->w_frame;
 
+  frame_locked++;
+
   // Save the position of the containing frame (which will also contain the
   // altframe) before we remove anything, to recompute window positions later.
   const win_T *const topleft = frame2win(frp_close->fr_parent);
@@ -3307,6 +3353,8 @@ win_T *winframe_remove(win_T *win, int *dirp, tabpage_T *tp, frame_T **unflat_al
     *unflat_altfr = altfr;
   }
 
+  frame_locked--;
+
   return wp;
 }
 
@@ -4315,6 +4363,9 @@ int win_new_tabpage(int after, char *filename)
     emsg(_(e_cmdwin));
     return FAIL;
   }
+  if (window_layout_locked()) {
+    return FAIL;
+  }
 
   tabpage_T *newtp = alloc_tabpage();
 

test/functional/ui/diff_spec.lua

@@ -3350,3 +3350,32 @@ describe("'diffanchors'", function()
     ]])
   end)
 end)
+
+-- oldtest: Test_diffexpr_wipe_buffers()
+it(':%bwipe does not crash when using diffexpr', function()
+  local screen = Screen.new(70, 20)
+  exec([[
+    func DiffFuncExpr()
+      let in = readblob(v:fname_in)
+      let new = readblob(v:fname_new)
+      let out = v:lua.vim.text.diff(in, new)
+      call writefile(split(out, "\n"), v:fname_out)
+    endfunc
+
+    new
+    vnew
+    set diffexpr=DiffFuncExpr()
+    wincmd l
+    new
+    call setline(1,range(20))
+    windo diffthis
+    wincmd w
+    hide
+    %bw!
+  ]])
+  screen:expect([[
+    ^                                                                      |
+    {1:~                                                                     }|*18
+    4 buffers wiped out                                                   |
+  ]])
+end)

test/old/testdir/test_arglist.vim

@@ -776,7 +776,6 @@ func Test_crash_arglist_uaf()
   "%argdelete
   new one
   au BufAdd XUAFlocal :bw
-  "call assert_fails(':arglocal XUAFlocal', 'E163:')
   arglocal XUAFlocal
   au! BufAdd
   bw! XUAFlocal
@@ -792,4 +791,15 @@ func Test_crash_arglist_uaf()
   au! BufAdd
 endfunc
 
+" This was using freed memory again
+func Test_crash_arglist_uaf2()
+  new
+  au BufAdd XUAFlocal :bw
+  arglocal XUAFlocal
+  redraw!
+  put ='abc'
+  2#
+  au! BufAdd
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

test/old/testdir/test_diffmode.vim

@@ -3311,4 +3311,36 @@ func Test_diff_add_prop_in_autocmd()
   call term_sendkeys(buf, ":diffsplit Xdiffsplit_file\<CR>")
   call VerifyScreenDump(buf, 'Test_diff_add_prop_in_autocmd_01', {})
 
+  call StopVimInTerminal(buf)
+endfunc
+
+" this was causing a use-after-free by callig winframe_remove() rerursively
+func Test_diffexpr_wipe_buffers()
+  CheckRunVimInTerminal
+
+  let lines =<< trim END
+    def DiffFuncExpr()
+      var in: list<string> = readfile(v:fname_in)
+      var new = readfile(v:fname_new)
+      var out: string = diff(in, new)
+      writefile(split(out, "n"), v:fname_out)
+    enddef
+
+    new
+    vnew
+    set diffexpr=DiffFuncExpr()
+    wincmd l
+    new
+    cal setline(1,range(20))
+    wind difft
+    wincm w
+    hid
+    %bw!
+  END
+  call writefile(lines, 'Xtest_diffexpr_wipe', 'D')
+
+  let buf = RunVimInTerminal('Xtest_diffexpr_wipe', {})
+  call term_sendkeys(buf, ":so\<CR>")
+  call WaitForAssert({-> assert_match('4 buffers wiped out', term_getline(buf, 20))})
+
   call StopVimInTerminal(buf)

test/old/testdir/test_filetype.vim

@@ -1196,6 +1196,34 @@ func Test_filetype_indent_off()
   close
 endfunc
 
+func Test_undo_ftplugin_on_buffer_reuse()
+  filetype on
+
+  new
+  let b:undo_ftplugin = ":let g:var='exists'"
+  let g:bufnr = bufnr('%')
+  " no changes done to the buffer, so the buffer will be re-used
+  e $VIMRUNTIME/defaults.vim
+  call assert_equal(g:bufnr, bufnr('%'))
+  call assert_equal('exists', get(g:, 'var', 'fail'))
+  unlet! g:bufnr g:var
+
+  " try to wipe the buffer
+  enew
+  bw defaults.vim
+  let b:undo_ftplugin = ':bw'
+  call assert_fails(':e $VIMRUNTIME/defaults.vim', 'E937:')
+
+  " try to split the window
+  enew
+  bw defaults.vim
+  let b:undo_ftplugin = ':sp $VIMRUNTIME/defaults.vim'
+  call assert_fails(':e $VIMRUNTIME/defaults.vim', 'E242:')
+
+  bwipe!
+  filetype off
+endfunc
+
 """""""""""""""""""""""""""""""""""""""""""""""""
 " Tests for specific extensions and filetypes.
 " Keep sorted.