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
Yes. I found after running my regular tests. Sorry I didn't mention
this.
--
Bad programs can be written in any language.
/// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
