Bram Moolenaar wrote:
> Dominique Pelle wrote:
>
>> >> I can reproduce a bug (use of freed memory) in Vim-7.2.284
>> >> on Linux x86 as follows:
>> >>
>> >> 1/ Start vim with valgrind:
>> >>
>> >> $ cd vim7/src
>> >> $ valgrind --leak-check=yes \
>> >> --num-callers=50 ./vim --noplugin -u NONE 2> vg.log
>> >>
>> >> 2/ Enter the 2 following Ex commands:
>> >>
>> >> :redir @"
>> >> :reg
>> >>
>> >> 3/ Observe in vg.log the following errors as soon as :reg is being
>> >> executed:
>> >>
>> >> ==13408== Invalid read of size 1
>> >> ==13408== at 0x40276F8: memmove (mc_replace_strmem.c:517)
>> >> ==13408== by 0x813CDA1: str_to_reg (ops.c:6157)
>> >> ==13408== by 0x813CB6F: write_reg_contents_ex (ops.c:6052)
>> >> ==13408== by 0x813C9E1: write_reg_contents (ops.c:5981)
>> >> ==13408== by 0x8104C0E: redir_write (message.c:3046)
>> >> ==13408== by 0x8103034: msg_puts_attr_len (message.c:1803)
>> >> ==13408== by 0x8102715: msg_outtrans_len_attr (message.c:1402)
>> >> ==13408== by 0x810243D: msg_outtrans_len (message.c:1291)
>> >> ==13408== by 0x81398A3: ex_display (ops.c:4013)
>> >> ==13408== by 0x80A7548: do_one_cmd (ex_docmd.c:2627)
>> >> ==13408== by 0x80A4D7F: do_cmdline (ex_docmd.c:1096)
>> >> ==13408== by 0x8090CB8: call_user_func (eval.c:21292)
>> >> ==13408== by 0x807CE17: call_func (eval.c:8123)
>> >> ==13408== by 0x807CA5B: get_func_tv (eval.c:7969)
>> >> ==13408== by 0x8078DF3: eval7 (eval.c:5021)
>> >> ==13408== by 0x80786FC: eval6 (eval.c:4688)
>> >> ==13408== by 0x80782E8: eval5 (eval.c:4504)
>> >> ==13408== by 0x8077839: eval4 (eval.c:4199)
>> >> ==13408== by 0x8077691: eval3 (eval.c:4111)
>> >> ==13408== by 0x807751D: eval2 (eval.c:4040)
>> >> ==13408== by 0x807734D: eval1 (eval.c:3965)
>> >> ==13408== by 0x80772B4: eval0 (eval.c:3922)
>> >> ==13408== by 0x8073AC5: ex_let (eval.c:1837)
>> >> ==13408== by 0x80A7548: do_one_cmd (ex_docmd.c:2627)
>> >> ==13408== by 0x80A4D7F: do_cmdline (ex_docmd.c:1096)
>> >> ==13408== by 0x80A2FE3: do_source (ex_cmds2.c:3116)
>> >> ==13408== by 0x80EA44D: source_startup_scripts (main.c:2778)
>> >> ==13408== by 0x80E74DB: main (main.c:563)
>> >> ==13408== Address 0x548108c is 4 bytes inside a block of size 32 free'd
>> >> ==13408== at 0x4024E5A: free (vg_replace_malloc.c:323)
>> >> ==13408== by 0x8116C67: vim_free (misc2.c:1639)
>> >> ==13408== by 0x813CD7A: str_to_reg (ops.c:6155)
>> >> ==13408== by 0x813CB6F: write_reg_contents_ex (ops.c:6052)
>> >> ==13408== by 0x813C9E1: write_reg_contents (ops.c:5981)
>> >> ==13408== by 0x8104C0E: redir_write (message.c:3046)
>> >> ==13408== by 0x8103034: msg_puts_attr_len (message.c:1803)
>> >> ==13408== by 0x8102715: msg_outtrans_len_attr (message.c:1402)
>> >> ==13408== by 0x810243D: msg_outtrans_len (message.c:1291)
>> >> ==13408== by 0x81398A3: ex_display (ops.c:4013)
>> >> ==13408== by 0x80A7548: do_one_cmd (ex_docmd.c:2627)
>> >> ==13408== by 0x80A4D7F: do_cmdline (ex_docmd.c:1096)
>> >> ==13408== by 0x8090CB8: call_user_func (eval.c:21292)
>> >> ==13408== by 0x807CE17: call_func (eval.c:8123)
>> >> ==13408== by 0x807CA5B: get_func_tv (eval.c:7969)
>> >> ==13408== by 0x8078DF3: eval7 (eval.c:5021)
>> >> ==13408== by 0x80786FC: eval6 (eval.c:4688)
>> >> ==13408== by 0x80782E8: eval5 (eval.c:4504)
>> >> ==13408== by 0x8077839: eval4 (eval.c:4199)
>> >> ==13408== by 0x8077691: eval3 (eval.c:4111)
>> >> ==13408== by 0x807751D: eval2 (eval.c:4040)
>> >> ==13408== by 0x807734D: eval1 (eval.c:3965)
>> >> ==13408== by 0x80772B4: eval0 (eval.c:3922)
>> >> ==13408== by 0x8073AC5: ex_let (eval.c:1837)
>> >> ==13408== by 0x80A7548: do_one_cmd (ex_docmd.c:2627)
>> >> ==13408== by 0x80A4D7F: do_cmdline (ex_docmd.c:1096)
>> >> ==13408== by 0x80A2FE3: do_source (ex_cmds2.c:3116)
>> >> ==13408== by 0x80EA44D: source_startup_scripts (main.c:2778)
>> >> ==13408== by 0x80E74DB: main (main.c:563)
>> >> (several more errors follow after that)
>> >>
>> >> The bug happens because function ex_display() is printing
>> >> all registers and while doing so, a register can be modified
>> >> if output is redirected to register (causing access to freed
>> >> memory).
>> >>
>> >> Attached patch fixes it by making function ex_display()
>> >> output a copy of the register. Please review it.
>> >>
>> >> I noticed this issue when trying Tony's .vimrc available at:
>> >> http://vim.wikia.com/wiki/User:Tonymec/vimrc
>> >>
>> >> The bug happens in function TestForX() in his vimrc file.
>> >
>> > The patch introduces quite a bit of new code, copies text around and
>> > allocates memory.
>> >
>> > How about this solution instead:
>> >
>> > *** ../vim-7.2.289/src/ops.c 2009-11-03 16:44:04.000000000 +0100
>> > --- src/ops.c 2009-11-11 16:13:33.000000000 +0100
>> > ***************
>> > *** 3990,3995 ****
>> > --- 3990,4001 ----
>> > }
>> > else
>> > yb = &(y_regs[i]);
>> > +
>> > + if (name == vim_tolower(redir_reg)
>> > + || (redir_reg == '"' && yb == y_previous))
>> > + continue; /* do not list register being written to, the
>> > + * pointer can be freed */
>> > +
>> > if (yb->y_array != NULL)
>> > {
>> > msg_putchar('\n');
>>
>>
>> Your patch is simpler (which is better) and I verified that it
>> also avoids access to freed memory.
>>
>> My patch and your patch have different outcomes though.
>>
>> Using the following script...
>>
>> :let @a = "foobar"
>> :redir @a
>> :sil reg a
>> :redir END
>> :echo @a
>>
>> With your patch, the echo statements prints one line:
>>
>> --- Registers ---
>>
>> With my patch, the echo statement prints 2 lines:
>>
>> --- Registers ---
>> "a ^J--- Registers ---
>>
>> I don't mind about the difference. In fact not outputting
>> the redirected register (as in your patch) is better in my
>> opinion since content of register @a has already been
>> lost as soon as we do ":redir @a".
>
> Thanks for checking the patch. Yes, the register written to is omitted
> from the list. I think that's somewhat better than listing a partly
> filled register. Definitely better than crashing.
Hi
I just realize that there is a problem with your proposed patch:
vim-tiny fails to compile.
ops.c: In function ‘ex_display’:
ops.c:3995: error: ‘redir_reg’ undeclared (first use in this function)
We need to add #ifdef FEAT_EVAL
RCS file: /cvsroot/vim/vim7/src/ops.c,v
retrieving revision 1.77
diff -c -r1.77 ops.c
*** src/ops.c 11 Nov 2009 16:22:28 -0000 1.77
--- src/ops.c 14 Nov 2009 19:51:22 -0000
***************
*** 3991,3996 ****
--- 3991,4004 ----
}
else
yb = &(y_regs[i]);
+
+ #ifdef FEAT_EVAL
+ if (name == vim_tolower(redir_reg)
+ || (redir_reg == '"' && yb == y_previous))
+ continue; /* do not list register being written to, the
+ * pointer can be freed */
+ #endif
+
if (yb->y_array != NULL)
{
msg_putchar('\n');
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
