commit 40fb2818b636b4d069a5bf6d16dff5d0f62a1775
parent 6fa2ebec6b244eea8dde01ca72deeebd4546e537
Author: zeertzjq <zeertzjq@outlook.com>
Date: Wed, 14 Jan 2026 09:43:48 +0800
vim-patch:9.1.2085: Use-after-free in winframe_remove()
Problem: Use-after-free in winframe_remove() (henices)
Solution: Set window_layout_locked() inside winframe_remove()
and check that writing diff files is disallowed when the
window layout is locked.
It can happen with a custom diff expression when removing a window:
1. Buffer was removed, so win_frame_remove() is called to remove the
window.
2. win_frame_remove() → frame_new_height() → scroll_to_fraction()
→ diff_check_fill() (checks for filler lines)
3. diff_check_fill() ends up causing a diff_try_update, and because we
are not using internal diff, it has to first write the file to a
buffer using buf_write()
4. buf_write() is called for a buffer that is not contained within a
window, so it first calls aucmd_prepbuf() to create a new temporary
window before writing the buffer and then later calls
aucmd_restbuf(), which restores the previous window layout, calling
winframe_remove() again, which will free the window/frame structure,
eventually freeing stuff that will still be accessed at step 2.
closes: vim/vim#19064
https://github.com/vim/vim/commit/ead1dda74a485ef0470e7252d07c1a36b8cde517
Nvim doesn't have this bug as Nvim uses a floating window as autocommand
window, and removing it doesn't need winframe_remove().
Co-authored-by: Christian Brabandt <cb@256bit.org>
Diffstat:
4 files changed, 82 insertions(+), 0 deletions(-)
diff --git a/src/nvim/diff.c b/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;
}
diff --git a/src/nvim/window.c b/src/nvim/window.c
@@ -112,6 +112,10 @@ static int split_disallowed = 0;
/// 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
@@ -129,6 +133,11 @@ void window_layout_unlock(void)
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)
{
@@ -3306,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);
@@ -3342,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;
}
diff --git a/test/functional/ui/diff_spec.lua b/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)
diff --git a/test/old/testdir/test_diffmode.vim b/test/old/testdir/test_diffmode.vim
@@ -3311,4 +3311,36 @@ func Test_diff_add_prop_in_autocmd()
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)
+endfunc
+
" vim: shiftwidth=2 sts=2 expandtab
