[patch] fixed access to freed memory when redirecting :reg to register

[Dominique Pellé]

Hi

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.

Cheers
-- Dominique

--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---

---

Index: ops.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/ops.c,v
retrieving revision 1.76
diff -c -r1.76 ops.c
*** ops.c 3 Nov 2009 15:44:21 -0000 1.76
--- ops.c 7 Nov 2009 17:13:42 -0000
***************
*** 990,995 ****
--- 990,1039 ----
  }
 
  /*
+  * Return a deep copy of register reg.  Memory is dynamically allocated,
+  * use free_register() to free the copy.
+  */
+     static struct yankreg *
+ copy_register(reg)
+     struct yankreg* reg;
+ {
+     struct yankreg *copy;
+     int i;
+
+     copy = (struct yankreg *)alloc((unsigned)sizeof(struct yankreg));
+     if (copy == NULL)
+ return NULL;
+
+     *copy = *reg;
+     copy->y_array = (char_u **)alloc((unsigned)reg->y_size*sizeof(char *));
+     if (copy->y_array == NULL)
+     {
+ vim_free(copy);
+ return NULL;
+     }
+     for (i = 0; i < reg->y_size; ++i)
+ copy->y_array[i] = vim_strsave(reg->y_array[i]);
+
+     return copy;
+ }
+
+ /*
+  * Free a register previous allocated with copy_register().
+  */
+     static void
+ free_register(reg)
+     struct yankreg* reg;
+ {
+     int i;
+
+     for (i = 0; i < reg->y_size; ++i)
+ vim_free(reg->y_array[i]);
+     vim_free(reg->y_array);
+     vim_free(reg);
+ }
+
+
+ /*
   * Put "reg" into register "name".  Free any previous contents and "reg".
   */
      void
***************
*** 3952,3957 ****
--- 3996,4002 ----
      long j;
      char_u *p;
      struct yankreg *yb;
+     struct yankreg      *copy_reg;
      int name;
      int attr;
      char_u *arg = eap->arg;
***************
*** 3990,4025 ****
  }
  else
     yb = &(y_regs[i]);
  if (yb->y_array != NULL)
  {
!    msg_putchar('\n');
!    msg_putchar('"');
!    msg_putchar(name);
!    MSG_PUTS("   ");
 
!    n = (int)Columns - 6;
!    for (j = 0; j < yb->y_size && n > 1; ++j)
!    {
! if (j)
! {
!    MSG_PUTS_ATTR("^J", attr);
!    n -= 2;
! }
! for (p = yb->y_array[j]; *p && (n -= ptr2cells(p)) >= 0; ++p)
  {
  #ifdef FEAT_MBYTE
!    clen = (*mb_ptr2len)(p);
  #endif
!    msg_outtrans_len(p, clen);
  #ifdef FEAT_MBYTE
!    p += clen - 1;
  #endif
  }
     }
!    if (n > 1 && yb->y_type == MLINE)
! MSG_PUTS_ATTR("^J", attr);
!    out_flush();    /* show one line at a time */
  }
  ui_breakcheck();
      }
 
--- 4035,4080 ----
  }
  else
     yb = &(y_regs[i]);
+
  if (yb->y_array != NULL)
  {
!    /* Output a copy of register yb, rather than register yb itself
!     * since yb may be overwritten while outputting it in case of
!     * redirection to register. */
!    copy_reg = copy_register(yb);
!    if (copy_reg != NULL)
!    {
! msg_putchar('\n');
! msg_putchar('"');
! msg_putchar(name);
! MSG_PUTS("   ");
 
! n = (int)Columns - 6;
! for (j = 0; j < copy_reg->y_size && n > 1; ++j)
  {
+    if (j)
+    {
+ MSG_PUTS_ATTR("^J", attr);
+ n -= 2;
+    }
+    for (p = copy_reg->y_array[j]; *p && (n -= ptr2cells(p)) >= 0; ++p)
+    {
  #ifdef FEAT_MBYTE
! clen = (*mb_ptr2len)(p);
  #endif
! msg_outtrans_len(p, clen);
  #ifdef FEAT_MBYTE
! p += clen - 1;
  #endif
+    }
  }
+ if (n > 1 && copy_reg->y_type == MLINE)
+    MSG_PUTS_ATTR("^J", attr);
+ out_flush();    /* show one line at a time */
     }
!    free_register(copy_reg);
  }
+
  ui_breakcheck();
      }
 
***************
*** 6089,6095 ****
      long maxlen;
  #endif
 
!     if (y_ptr->y_array == NULL) /* NULL means emtpy register */
  y_ptr->y_size = 0;
 
      /*
--- 6144,6150 ----
      long maxlen;
  #endif
 
!     if (y_ptr->y_array == NULL) /* NULL means empty register */
  y_ptr->y_size = 0;
 
      /*

---

[Bram Moolenaar]

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.

Thanks!


--
No children may attend school with their breath smelling of "wild onions."
                [real standing law in West Virginia, United States of America]

 /// Bram Moolenaar -- Bram@... -- 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
-~----------~----~----~----~------~----~------~--~---

[Bram Moolenaar]

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');


--
Mynd you, m00se bites Kan be pretty nasti ...
                 "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

 /// Bram Moolenaar -- Bram@... -- 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
-~----------~----~----~----~------~----~------~--~---

[Dominique Pellé]

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

[Bram Moolenaar]

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 -- Bram@... -- 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
-~----------~----~----~----~------~----~------~--~---

[Dominique Pellé]

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

[Bram Moolenaar]

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 -- Bram@... -- 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
-~----------~----~----~----~------~----~------~--~---