mirror of
https://github.com/neovim/neovim.git
synced 2024-09-17 20:58:20 -04:00
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
This commit is contained in:
parent
9f29176033
commit
eb4676c67f
@ -2763,11 +2763,14 @@ nvim_set_decoration_provider({ns_id}, {*opts})
|
||||
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
|
||||
experimental. 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.
|
||||
|
||||
Attributes: ~
|
||||
Lua |vim.api| only
|
||||
|
||||
|
@ -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;
|
||||
}
|
||||
|
||||
|
@ -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
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -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 });
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
@ -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)
|
||||
|
@ -122,7 +122,7 @@ describe('decorations providers', function()
|
||||
]]}
|
||||
check_trace {
|
||||
{ "start", 5 };
|
||||
{ "buf", 1 };
|
||||
{ "buf", 1, 5 };
|
||||
{ "win", 1000, 1, 0, 8 };
|
||||
{ "line", 1000, 1, 6 };
|
||||
{ "end", 5 };
|
||||
@ -565,6 +565,23 @@ describe('decorations providers', function()
|
||||
|
|
||||
]])
|
||||
end)
|
||||
|
||||
it('does not allow removing extmarks during on_line callbacks', function()
|
||||
exec_lua([[
|
||||
eok = true
|
||||
]])
|
||||
setup_provider([[
|
||||
local function on_do(kind, winid, bufnr, topline, botline_guess)
|
||||
if kind == 'line' then
|
||||
api.nvim_buf_set_extmark(bufnr, ns1, 1, -1, { sign_text = 'X' })
|
||||
eok = pcall(api.nvim_buf_clear_namespace, bufnr, ns1, 0, -1)
|
||||
end
|
||||
end
|
||||
]])
|
||||
exec_lua([[
|
||||
assert(eok == false)
|
||||
]])
|
||||
end)
|
||||
end)
|
||||
|
||||
describe('extmark decorations', function()
|
||||
|
Loading…
Reference in New Issue
Block a user