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". Cheers -- Dominique --~--~---------~--~----~------------~-------~--~----~ You received this message from the "vim_dev" maillist. For more information, visit http://www.vim.org/maillist.php -~----------~----~----~----~------~----~------~--~---
