[bug] another use of free memory in quickfix code

[Dominique Pellé]

Hi

afl-fuzz found another crash with use of freed memory in vim-7.4.1627
(and older) in quickfix code. This is a different bug that the one fixed
recently in Vim-7.4.1592:

$ cat > crash.vim <<EOF
fun! R()
lolder
lgetexpr []
endfun

autocmd BufReadCmd * call R()
let words = [ "a", "b" ]
let qflist = []
for word in words
call add(qflist, {'filename': 't'})
call setloclist(0, qflist, ' ')
endfor
lrewind
EOF

$ vim -u NONE -N -S crash.vim
Vim: Caught deadly signal SEGV

Vim: Finished.
Segmentation fault


Valgrind says:

==28906== Memcheck, a memory error detector
==28906== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==28906== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==28906== Command: ./vim -u NONE -N -S c.vim
==28906==
==28906== Invalid read of size 8
==28906== at 0x53891F: qf_jump (quickfix.c:1817)
==28906== by 0x53B793: ex_cc (quickfix.c:3150)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x47073B: do_source (ex_cmds2.c:3896)
==28906== by 0x46FD43: cmd_source (ex_cmds2.c:3509)
==28906== by 0x46FC94: ex_source (ex_cmds2.c:3484)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x472081: do_cmdline_cmd (ex_docmd.c:712)
==28906== by 0x5F5D1E: exe_commands (main.c:2927)
==28906== by 0x5F320B: main (main.c:955)
==28906== Address 0x791e0a8 is 40 bytes inside a block of size 64 free'd
==28906== at 0x4C2CE2B: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28906== by 0x4ED684: vim_free (misc2.c:1695)
==28906== by 0x539723: qf_free (quickfix.c:2154)
==28906== by 0x536DCB: qf_new_list (quickfix.c:915)
==28906== by 0x534EF4: qf_init_ext (quickfix.c:266)
==28906== by 0x53DBD6: ex_cexpr (quickfix.c:4173)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x458E73: call_user_func (eval.c:24968)
==28906== by 0x43AD50: call_func (eval.c:8897)
==28906== by 0x43A622: get_func_tv (eval.c:8686)
==28906== by 0x431FD9: ex_call (eval.c:3536)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x4A338E: apply_autocmds_group (fileio.c:9451)
==28906== by 0x4A2B18: apply_autocmds_exarg (fileio.c:9018)
==28906== by 0x495847: readfile (fileio.c:375)
==28906== by 0x4059F1: open_buffer (buffer.c:151)
==28906== by 0x461EC9: do_ecmd (ex_cmds.c:3768)
==28906== by 0x461265: getfile (ex_cmds.c:3190)
==28906== by 0x408A7D: buflist_getfile (buffer.c:2094)
==28906== by 0x538899: qf_jump (quickfix.c:1797)
==28906== by 0x53B793: ex_cc (quickfix.c:3150)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x47073B: do_source (ex_cmds2.c:3896)
==28906== by 0x46FD43: cmd_source (ex_cmds2.c:3509)
==28906== by 0x46FC94: ex_source (ex_cmds2.c:3484)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x472081: do_cmdline_cmd (ex_docmd.c:712)
==28906== by 0x5F5D1E: exe_commands (main.c:2927)
==28906== by 0x5F320B: main (main.c:955)
==28906== Block was alloc'd at
==28906== at 0x4C2BBCF: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28906== by 0x4ECAC5: lalloc (misc2.c:920)
==28906== by 0x4EC992: alloc (misc2.c:818)
==28906== by 0x537000: qf_add_entry (quickfix.c:998)
==28906== by 0x53D6BD: set_errorlist (quickfix.c:4042)
==28906== by 0x44AB6E: set_qf_ll_list (eval.c:17940)
==28906== by 0x44ABE0: f_setloclist (eval.c:17959)
==28906== by 0x43AECE: call_func (eval.c:8927)
==28906== by 0x43A622: get_func_tv (eval.c:8686)
==28906== by 0x431FD9: ex_call (eval.c:3536)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x47073B: do_source (ex_cmds2.c:3896)
==28906== by 0x46FD43: cmd_source (ex_cmds2.c:3509)
==28906== by 0x46FC94: ex_source (ex_cmds2.c:3484)
==28906== by 0x475E5D: do_one_cmd (ex_docmd.c:2921)
==28906== by 0x472A45: do_cmdline (ex_docmd.c:1107)
==28906== by 0x472081: do_cmdline_cmd (ex_docmd.c:712)
==28906== by 0x5F5D1E: exe_commands (main.c:2927)
==28906== by 0x5F320B: main (main.c:955)
(more errors after that)

Regards
Dominique

[Yegappan Lakshmanan]

Hi,

On Mon, Mar 21, 2016 at 2:45 AM, Dominique Pellé
<dominiq...@gmail.com> wrote:
> Hi
>
> afl-fuzz found another crash with use of freed memory in vim-7.4.1627
> (and older) in quickfix code. This is a different bug that the one fixed
> recently in Vim-7.4.1592:
>

The attached patch fixes this issue. These issues are caused by autocmds
modifying the location list while the location list is being used.

- Yegappan

[h_east]

Hi Yegappan,

2016-3-23(Wed) 14:02:53 UTC+9 yega...@gmail.com:

> Hi,
>
> On Mon, Mar 21, 2016 at 2:45 AM, Dominique Pellé
> <dominiq...@gmail.com> wrote:
> > Hi
> >
> > afl-fuzz found another crash with use of freed memory in vim-7.4.1627
> > (and older) in quickfix code. This is a different bug that the one fixed
> > recently in Vim-7.4.1592:
> >
>
> The attached patch fixes this issue. These issues are caused by autocmds
> modifying the location list while the location list is being used.

This issue also occurs with the quickfix.

]$ vim -Nu NONE -S crash_qf.vim

Vim: Caught deadly signal SEGV

Vim: Finished.

Segmentation fault (core dumped)


PS
I think we should abort even if `qi->qf_curlist` has changed.

I'm sorry. In fact, I have had already written a patch.
But did not yet been submitted.
I'll send patch later.

--
Best regards,
Hirohito Higashi (a.k.a. h_east)

[h_east]

Hi Yegappan and list,

2016-3-23(Wed) 14:39:31 UTC+9 h_east:

A patch is attached.
Please check it.

Yegappan>
I have modified to respect your patch.
Thank you for the nice patch.
After all, You are the quickfix wizard :-)

[Yegappan Lakshmanan]

Hi,

On Wed, Mar 23, 2016 at 5:36 AM, h_east <h.eas...@gmail.com> wrote:
>
>> > On Mon, Mar 21, 2016 at 2:45 AM, Dominique Pellé
>> > <dominiq...@gmail.com> wrote:
>> > > Hi
>> > >
>> > > afl-fuzz found another crash with use of freed memory in vim-7.4.1627
>> > > (and older) in quickfix code. This is a different bug that the one fixed
>> > > recently in Vim-7.4.1592:
>> > >
>> >
>> > The attached patch fixes this issue. These issues are caused by autocmds
>> > modifying the location list while the location list is being used.
>>
>> This issue also occurs with the quickfix.
>>
>> ]$ vim -Nu NONE -S crash_qf.vim
>> Vim: Caught deadly signal SEGV
>>
>> Vim: Finished.
>> Segmentation fault (core dumped)
>>
>> PS
>> I think we should abort even if `qi->qf_curlist` has changed.
>>
>> I'm sorry. In fact, I have had already written a patch.
>> But did not yet been submitted.
>> I'll send patch later.
>
> A patch is attached.
> Please check it.
>

The patch looks good to me. One comment: When the quickfix/location
list is modified, the opened_window variable is set to FALSE even though
the window is still valid/open. I think, in this case the new window needs
to be closed. The opened_window variable should be set to FALSE
only when win_valid() returns FALSE.

- Yegappan

[Bram Moolenaar]

Thanks!

> Yegappan>
> I have modified to respect your patch.
> Thank you for the nice patch.
> After all, You are the quickfix wizard :-)



--

hundred-and-one symptoms of being an internet addict:
102. When filling out your driver's license application, you give
your IP address.

/// 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 ///

[h_east]

Hi Yegappan and Bram,

2016-3-23(Wed) 22:52:18 UTC+9 yega...@gmail.com:

Thanks for the comment.
While I'm sleeping, Bram fixed this comment's issue :-)

patch 7.4.1640
https://github.com/vim/vim/commit/ffec3c53496d49668669deabc0724ec78e2274fd

Thank you Bram!