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. -- SOLDIER: What? A swallow carrying a coconut? ARTHUR: It could grip it by the husk ... "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// 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 -~----------~----~----~----~------~----~------~--~---
