Patch 8.2.2421

[Bram Moolenaar]

Patch 8.2.2421
Problem: Double free when using autocommand with "argdel". (Houyunsong)
Solution: Add the arglist_locked flag.
Files: src/arglist.c, src/testdir/test_autocmd.vim


*** ../vim-8.2.2420/src/arglist.c 2021-01-26 21:35:05.048158026 +0100
--- src/arglist.c 2021-01-28 14:17:37.906356405 +0100
***************
*** 17,28 ****
--- 17,45 ----
#define AL_ADD 2
#define AL_DEL 3

+ // This flag is set whenever the argument list is being changed and calling a
+ // function that might trigger an autocommand.
+ static int arglist_locked = FALSE;
+
+ static int
+ check_arglist_locked(void)
+ {
+ if (arglist_locked)
+ {
+ emsg(_(e_cannot_change_arglist_recursively));
+ return FAIL;
+ }
+ return OK;
+ }
+
/*
* Clear an argument list: free all file names and reset it to zero entries.
*/
void
alist_clear(alist_T *al)
{
+ if (check_arglist_locked() == FAIL)
+ return;
while (--al->al_ga.ga_len >= 0)
vim_free(AARGLIST(al)[al->al_ga.ga_len].ae_fname);
ga_clear(&al->al_ga);
***************
*** 126,139 ****
int fnum_len)
{
int i;
- static int recursive = 0;

! if (recursive)
! {
! emsg(_(e_au_recursive));
return;
- }
- ++recursive;

alist_clear(al);
if (ga_grow(&al->al_ga, count) == OK)
--- 143,151 ----
int fnum_len)
{
int i;

! if (check_arglist_locked() == FAIL)
return;

alist_clear(al);
if (ga_grow(&al->al_ga, count) == OK)
***************
*** 152,158 ****
--- 164,174 ----
// May set buffer name of a buffer previously used for the
// argument list, so that it's re-used by alist_add.
if (fnum_list != NULL && i < fnum_len)
+ {
+ arglist_locked = TRUE;
buf_set_name(fnum_list[i], files[i]);
+ arglist_locked = FALSE;
+ }

alist_add(al, files[i], use_curbuf ? 2 : 1);
ui_breakcheck();
***************
*** 163,170 ****
FreeWild(count, files);
if (al == &global_alist)
arg_had_last = FALSE;
-
- --recursive;
}

/*
--- 179,184 ----
***************
*** 179,184 ****
--- 193,202 ----
{
if (fname == NULL) // don't add NULL file names
return;
+ if (check_arglist_locked() == FAIL)
+ return;
+ arglist_locked = TRUE;
+
#ifdef BACKSLASH_IN_FILENAME
slash_adjust(fname);
#endif
***************
*** 187,192 ****
--- 205,212 ----
AARGLIST(al)[al->al_ga.ga_len].ae_fnum =
buflist_add(fname, BLN_LISTED | (set_fnum == 2 ? BLN_CURBUF : 0));
++al->al_ga.ga_len;
+
+ arglist_locked = FALSE;
}

#if defined(BACKSLASH_IN_FILENAME) || defined(PROTO)
***************
*** 334,340 ****
int i;
int old_argcount = ARGCOUNT;

! if (ga_grow(&ALIST(curwin)->al_ga, count) == OK)
{
if (after < 0)
after = 0;
--- 354,361 ----
int i;
int old_argcount = ARGCOUNT;

! if (check_arglist_locked() != FAIL
! && ga_grow(&ALIST(curwin)->al_ga, count) == OK)
{
if (after < 0)
after = 0;
***************
*** 343,348 ****
--- 364,370 ----
if (after < ARGCOUNT)
mch_memmove(&(ARGLIST[after + count]), &(ARGLIST[after]),
(ARGCOUNT - after) * sizeof(aentry_T));
+ arglist_locked = TRUE;
for (i = 0; i < count; ++i)
{
int flags = BLN_LISTED | (will_edit ? BLN_CURBUF : 0);
***************
*** 350,355 ****
--- 372,378 ----
ARGLIST[after + i].ae_fname = files[i];
ARGLIST[after + i].ae_fnum = buflist_add(files[i], flags);
}
+ arglist_locked = FALSE;
ALIST(curwin)->al_ga.ga_len += count;
if (old_argcount > 0 && curwin->w_arg_idx >= after)
curwin->w_arg_idx += count;
***************
*** 382,387 ****
--- 405,413 ----
int match;
int arg_escaped = TRUE;

+ if (check_arglist_locked() == FAIL)
+ return FAIL;
+
// Set default argument for ":argadd" command.
if (what == AL_ADD && *str == NUL)
{
***************
*** 776,781 ****
--- 802,810 ----
int i;
int n;

+ if (check_arglist_locked() == FAIL)
+ return;
+
if (eap->addr_count > 0 || *eap->arg == NUL)
{
// ":argdel" works like ":.argdel"
*** ../vim-8.2.2420/src/testdir/test_autocmd.vim 2021-01-28 13:47:55.056905117 +0100
--- src/testdir/test_autocmd.vim 2021-01-28 14:19:59.002169881 +0100
***************
*** 173,178 ****
--- 173,184 ----
quit
endfunc

+ func Test_argdelete_in_next()
+ au BufNew,BufEnter,BufLeave,BufWinEnter * argdel
+ call assert_fails('next a b', 'E1156:')
+ au! BufNew,BufEnter,BufLeave,BufWinEnter *
+ endfunc
+
func Test_autocmd_bufwinleave_with_tabfirst()
tabedit
augroup sample
*** ../vim-8.2.2420/src/version.c 2021-01-28 13:47:55.060905101 +0100
--- src/version.c 2021-01-28 14:18:57.030234473 +0100
***************
*** 752,753 ****
--- 752,755 ----
{ /* Add new patch number below this line */
+ /**/
+ 2421,
/**/

--
hundred-and-one symptoms of being an internet addict:
256. You are able to write down over 250 symptoms of being an internet
addict, even though they only asked for 101.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Marius Gedminas]

On Thu, Jan 28, 2021 at 02:26:18PM +0100, Bram Moolenaar wrote:
> Patch 8.2.2421
> Problem: Double free when using autocommand with "argdel". (Houyunsong)
> Solution: Add the arglist_locked flag.
> Files: src/arglist.c, src/testdir/test_autocmd.vim

This fails to build with

arglist.c: In function ‘check_arglist_locked’:
arglist.c:29:9: error: ‘e_cannot_change_arglist_recursively’ undeclared (first use in this function); did you mean ‘e_cannot_change_list_item’?
29 | emsg(_(e_cannot_change_arglist_recursively));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Marius Gedminas
--
I can barely exit nano.
-- Christian Neukirchen