[bug] invalid memory access with normal commands
Hi afl-fuzz found another command that causes access to invalid memory in Vim-8.0.329. It's not a recent regression since bug is present in at least Vim-7.4.52 that comes with ubuntu-14.04. I have not been able to find a fix yet. Step to reproduce: $ valgrind vim -u NONE -c'norm oxx' -c'norm vapo' -c'q!' 2>vg.log And vg.log contains: ==4259== Memcheck, a memory error detector ==4259== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. ==4259== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info ==4259== Command: vim -u NONE -cnorm\ oxx -cnorm\ vapo -cq! ==4259== ==4259== Invalid read of size 1 ==4259==at 0x4CF4A0: utf_head_off (mbyte.c:3746) ==4259==by 0x4D094C: mb_adjustpos (mbyte.c:4028) ==4259==by 0x4E11C9: normal_cmd (normal.c:1292) ==4259==by 0x468F25: exec_normal (ex_docmd.c:10418) ==4259==by 0x469076: ex_normal (ex_docmd.c:10310) ==4259==by 0x46B734: do_one_cmd (ex_docmd.c:2981) ==4259==by 0x46B734: do_cmdline (ex_docmd.c:1120) ==4259==by 0x5BFC2B: exe_commands (main.c:2905) ==4259==by 0x5BFC2B: vim_main2 (main.c:781) ==4259==by 0x40C3D3: main (main.c:415) ==4259== Address 0xa95bc50 is 0 bytes after a block of size 4,096 alloc'd ==4259==at 0x4C2ABF5: malloc (vg_replace_malloc.c:299) ==4259==by 0x4C4D0B: lalloc (misc2.c:942) ==4259==by 0x5C0830: mf_alloc_bhdr.isra.3 (memfile.c:907) ==4259==by 0x5C1546: mf_new (memfile.c:381) ==4259==by 0x4AC6FF: ml_new_data (memline.c:3513) ==4259==by 0x4AF0FC: ml_open (memline.c:400) ==4259==by 0x414DD6: open_buffer (buffer.c:163) ==4259==by 0x5BF891: create_windows (main.c:2677) ==4259==by 0x5BF891: vim_main2 (main.c:704) ==4259==by 0x40C3D3: main (main.c:415) Possibly related to this, the following command does not seem to behave correctly, even if it does not trigger invalid memory: $ echo "foo bar" | vim -u NONE -c'norm wvapo' - Then press o multiple times and observe that cursor alternates between the beginning of the visual block and the *middle* of the visual block. I would expect it to alternate between the beginning and the *end* of the visual block. Regards Dominique -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [bug] invalid memory access with normal commands
Hi Dominique! On Di, 14 Feb 2017, Dominique Pellé wrote: > Hi > > afl-fuzz found another command that causes access to > invalid memory in Vim-8.0.329. It's not a recent regression > since bug is present in at least Vim-7.4.52 that comes > with ubuntu-14.04. > > I have not been able to find a fix yet. > > Step to reproduce: > > $ valgrind vim -u NONE -c'norm oxx' -c'norm vapo' -c'q!' 2>vg.log > > And vg.log contains: > > ==4259== Memcheck, a memory error detector > ==4259== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. > ==4259== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright > info > ==4259== Command: vim -u NONE -cnorm\ oxx -cnorm\ vapo -cq! > ==4259== > ==4259== Invalid read of size 1 > ==4259==at 0x4CF4A0: utf_head_off (mbyte.c:3746) > ==4259==by 0x4D094C: mb_adjustpos (mbyte.c:4028) > ==4259==by 0x4E11C9: normal_cmd (normal.c:1292) > ==4259==by 0x468F25: exec_normal (ex_docmd.c:10418) > ==4259==by 0x469076: ex_normal (ex_docmd.c:10310) > ==4259==by 0x46B734: do_one_cmd (ex_docmd.c:2981) > ==4259==by 0x46B734: do_cmdline (ex_docmd.c:1120) > ==4259==by 0x5BFC2B: exe_commands (main.c:2905) > ==4259==by 0x5BFC2B: vim_main2 (main.c:781) > ==4259==by 0x40C3D3: main (main.c:415) > ==4259== Address 0xa95bc50 is 0 bytes after a block of size 4,096 alloc'd > ==4259==at 0x4C2ABF5: malloc (vg_replace_malloc.c:299) > ==4259==by 0x4C4D0B: lalloc (misc2.c:942) > ==4259==by 0x5C0830: mf_alloc_bhdr.isra.3 (memfile.c:907) > ==4259==by 0x5C1546: mf_new (memfile.c:381) > ==4259==by 0x4AC6FF: ml_new_data (memline.c:3513) > ==4259==by 0x4AF0FC: ml_open (memline.c:400) > ==4259==by 0x414DD6: open_buffer (buffer.c:163) > ==4259==by 0x5BF891: create_windows (main.c:2677) > ==4259==by 0x5BF891: vim_main2 (main.c:704) > ==4259==by 0x40C3D3: main (main.c:415) > > Possibly related to this, the following command does not > seem to behave correctly, even if it does not trigger invalid > memory: > > $ echo "foo bar" | vim -u NONE -c'norm wvapo' - > > Then press o multiple times and observe that cursor > alternates between the beginning of the visual block > and the *middle* of the visual block. I would expect it to > alternate between the beginning and the *end* of the > visual block. Interesting. The second issue happens, because current_par does internally switch to visual line mode, but does not reset the VIsual column correctly. This patch fixes it: diff --git a/src/search.c b/src/search.c index 36410e50f..ffda65837 100644 --- a/src/search.c +++ b/src/search.c @@ -4241,6 +4241,7 @@ extend: * line, we get stuck there. Trap this here. */ if (VIsual_mode == 'V' && start_lnum == curwin->w_cursor.lnum) goto extend; + VIsual.col = 0; VIsual.lnum = start_lnum; VIsual_mode = 'V'; redraw_curbuf_later(INVERTED); /* update the inversion */ As a side effect this also disables switching cursor position using 'o' which is currently a no-op when only a single line is selected (since it effectively switches positions between the start of line and the start of line). Interestingly, I never noticed that this does nothing currently if a single visual line is selected. As it stands, I think this also fixes the invalid read mentioned above. I just wanted to take a look at that issue, when I noticed that this patch does not exhibit this behaviour, so I didn't check very closely why it works. I tried to add a test, to verify that this is fixed, but whatever I do, I always get back col 0 for the visual mode. I think this is because in getmark_buf_fnum() (mark.c:417) the column is explicitly set to zero for visual line mode and also using getpos('v') does not work, since by the time I run it, Visual mode is no longer active, so I cannot verify that this works in vimscript. Best, Christian -- Mein Freund, ich brauche dich - wie eine Höhe, in der man anders atmet. -- Antoine de Saint-Exupéry -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [bug] invalid memory access with normal commands
Christian Brabandt wrote: > Hi Dominique! > > On Di, 14 Feb 2017, Dominique Pellé wrote: > >> Hi >> >> afl-fuzz found another command that causes access to >> invalid memory in Vim-8.0.329. It's not a recent regression >> since bug is present in at least Vim-7.4.52 that comes >> with ubuntu-14.04. >> >> I have not been able to find a fix yet. >> >> Step to reproduce: >> >> $ valgrind vim -u NONE -c'norm oxx' -c'norm vapo' -c'q!' 2>vg.log >> >> And vg.log contains: >> >> ==4259== Memcheck, a memory error detector >> ==4259== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. >> ==4259== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright >> info >> ==4259== Command: vim -u NONE -cnorm\ oxx -cnorm\ vapo -cq! >> ==4259== >> ==4259== Invalid read of size 1 >> ==4259==at 0x4CF4A0: utf_head_off (mbyte.c:3746) >> ==4259==by 0x4D094C: mb_adjustpos (mbyte.c:4028) >> ==4259==by 0x4E11C9: normal_cmd (normal.c:1292) >> ==4259==by 0x468F25: exec_normal (ex_docmd.c:10418) >> ==4259==by 0x469076: ex_normal (ex_docmd.c:10310) >> ==4259==by 0x46B734: do_one_cmd (ex_docmd.c:2981) >> ==4259==by 0x46B734: do_cmdline (ex_docmd.c:1120) >> ==4259==by 0x5BFC2B: exe_commands (main.c:2905) >> ==4259==by 0x5BFC2B: vim_main2 (main.c:781) >> ==4259==by 0x40C3D3: main (main.c:415) >> ==4259== Address 0xa95bc50 is 0 bytes after a block of size 4,096 alloc'd >> ==4259==at 0x4C2ABF5: malloc (vg_replace_malloc.c:299) >> ==4259==by 0x4C4D0B: lalloc (misc2.c:942) >> ==4259==by 0x5C0830: mf_alloc_bhdr.isra.3 (memfile.c:907) >> ==4259==by 0x5C1546: mf_new (memfile.c:381) >> ==4259==by 0x4AC6FF: ml_new_data (memline.c:3513) >> ==4259==by 0x4AF0FC: ml_open (memline.c:400) >> ==4259==by 0x414DD6: open_buffer (buffer.c:163) >> ==4259==by 0x5BF891: create_windows (main.c:2677) >> ==4259==by 0x5BF891: vim_main2 (main.c:704) >> ==4259==by 0x40C3D3: main (main.c:415) >> >> Possibly related to this, the following command does not >> seem to behave correctly, even if it does not trigger invalid >> memory: >> >> $ echo "foo bar" | vim -u NONE -c'norm wvapo' - >> >> Then press o multiple times and observe that cursor >> alternates between the beginning of the visual block >> and the *middle* of the visual block. I would expect it to >> alternate between the beginning and the *end* of the >> visual block. > > Interesting. The second issue happens, because current_par does > internally switch to visual line mode, but does not reset the VIsual > column correctly. This patch fixes it: > > diff --git a/src/search.c b/src/search.c > index 36410e50f..ffda65837 100644 > --- a/src/search.c > +++ b/src/search.c > @@ -4241,6 +4241,7 @@ extend: > * line, we get stuck there. Trap this here. */ > if (VIsual_mode == 'V' && start_lnum == curwin->w_cursor.lnum) > goto extend; > + VIsual.col = 0; > VIsual.lnum = start_lnum; > VIsual_mode = 'V'; > redraw_curbuf_later(INVERTED); /* update the inversion */ > > As a side effect this also disables switching cursor position using 'o' > which is currently a no-op when only a single line is selected (since it > effectively switches positions between the start of line and the start > of line). > > Interestingly, I never noticed that this does nothing currently if a > single visual line is selected. > > As it stands, I think this also fixes the invalid read mentioned above. > I just wanted to take a look at that issue, when I noticed that this > patch does not exhibit this behaviour, so I didn't check very closely > why it works. > > I tried to add a test, to verify that this is fixed, but whatever I do, > I always get back col 0 for the visual mode. I think this is because in > getmark_buf_fnum() (mark.c:417) the column is explicitly set to zero for > visual line mode and also using getpos('v') does not work, since by the > time I run it, Visual mode is no longer active, so I cannot verify that > this works in vimscript. Hi Christian Your patch appears to fix the valgrind error. However, it's not correct. If I do... $ echo "foo bar" | vim -u NONE -c'norm wvapo' - ... then pressing o multiple times, the cursor remains at the beginning of the visual selection, instead of alternating at the beginning and at the end of the visual selection. Regards Dominique -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [bug] invalid memory access with normal commands
On Mi, 15 Feb 2017, Dominique Pellé wrote: > Christian Brabandt wrote: > > On Di, 14 Feb 2017, Dominique Pellé wrote: > Your patch appears to fix the valgrind error. > However, it's not correct. If I do... > > $ echo "foo bar" | vim -u NONE -c'norm wvapo' - > > ... then pressing o multiple times, the cursor remains > at the beginning of the visual selection, instead of > alternating at the beginning and at the end of the > visual selection. That's what I meant with "I did never notice that this is the exact same behaviour if you use V (e.g. only visually select a single line) and then press o" Best, Christian -- Wen das Wort nicht schlägt, den schlägt auch der Stock nicht. -- Sokrates (470-399 v.Chr.) -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [bug] invalid memory access with normal commands
Christian Brabandt wrote: > On Di, 14 Feb 2017, Dominique Pellé wrote: > > > Hi > > > > afl-fuzz found another command that causes access to > > invalid memory in Vim-8.0.329. It's not a recent regression > > since bug is present in at least Vim-7.4.52 that comes > > with ubuntu-14.04. > > > > I have not been able to find a fix yet. > > > > Step to reproduce: > > > > $ valgrind vim -u NONE -c'norm oxx' -c'norm vapo' -c'q!' 2>vg.log > > > > And vg.log contains: > > > > ==4259== Memcheck, a memory error detector > > ==4259== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al. > > ==4259== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright > > info > > ==4259== Command: vim -u NONE -cnorm\ oxx -cnorm\ vapo -cq! > > ==4259== > > ==4259== Invalid read of size 1 > > ==4259==at 0x4CF4A0: utf_head_off (mbyte.c:3746) > > ==4259==by 0x4D094C: mb_adjustpos (mbyte.c:4028) > > ==4259==by 0x4E11C9: normal_cmd (normal.c:1292) > > ==4259==by 0x468F25: exec_normal (ex_docmd.c:10418) > > ==4259==by 0x469076: ex_normal (ex_docmd.c:10310) > > ==4259==by 0x46B734: do_one_cmd (ex_docmd.c:2981) > > ==4259==by 0x46B734: do_cmdline (ex_docmd.c:1120) > > ==4259==by 0x5BFC2B: exe_commands (main.c:2905) > > ==4259==by 0x5BFC2B: vim_main2 (main.c:781) > > ==4259==by 0x40C3D3: main (main.c:415) > > ==4259== Address 0xa95bc50 is 0 bytes after a block of size 4,096 alloc'd > > ==4259==at 0x4C2ABF5: malloc (vg_replace_malloc.c:299) > > ==4259==by 0x4C4D0B: lalloc (misc2.c:942) > > ==4259==by 0x5C0830: mf_alloc_bhdr.isra.3 (memfile.c:907) > > ==4259==by 0x5C1546: mf_new (memfile.c:381) > > ==4259==by 0x4AC6FF: ml_new_data (memline.c:3513) > > ==4259==by 0x4AF0FC: ml_open (memline.c:400) > > ==4259==by 0x414DD6: open_buffer (buffer.c:163) > > ==4259==by 0x5BF891: create_windows (main.c:2677) > > ==4259==by 0x5BF891: vim_main2 (main.c:704) > > ==4259==by 0x40C3D3: main (main.c:415) > > > > Possibly related to this, the following command does not > > seem to behave correctly, even if it does not trigger invalid > > memory: > > > > $ echo "foo bar" | vim -u NONE -c'norm wvapo' - > > > > Then press o multiple times and observe that cursor > > alternates between the beginning of the visual block > > and the *middle* of the visual block. I would expect it to > > alternate between the beginning and the *end* of the > > visual block. > > Interesting. The second issue happens, because current_par does > internally switch to visual line mode, but does not reset the VIsual > column correctly. This patch fixes it: > > diff --git a/src/search.c b/src/search.c > index 36410e50f..ffda65837 100644 > --- a/src/search.c > +++ b/src/search.c > @@ -4241,6 +4241,7 @@ extend: > * line, we get stuck there. Trap this here. */ > if (VIsual_mode == 'V' && start_lnum == curwin->w_cursor.lnum) > goto extend; > + VIsual.col = 0; > VIsual.lnum = start_lnum; > VIsual_mode = 'V'; > redraw_curbuf_later(INVERTED); /* update the inversion */ > > As a side effect this also disables switching cursor position using 'o' > which is currently a no-op when only a single line is selected (since it > effectively switches positions between the start of line and the start > of line). I think the column should only be reset when VIsual.lnum actually changes. > Interestingly, I never noticed that this does nothing currently if a > single visual line is selected. > > As it stands, I think this also fixes the invalid read mentioned above. > I just wanted to take a look at that issue, when I noticed that this > patch does not exhibit this behaviour, so I didn't check very closely > why it works. > > I tried to add a test, to verify that this is fixed, but whatever I do, > I always get back col 0 for the visual mode. I think this is because in > getmark_buf_fnum() (mark.c:417) the column is explicitly set to zero for > visual line mode and also using getpos('v') does not work, since by the > time I run it, Visual mode is no longer active, so I cannot verify that > this works in vimscript. I can at least add a test to verify the memory access is fixed. -- "Beware of bugs in the above code; I have only proved it correct, not tried it." -- Donald Knuth /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net \\\ ///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org/// \\\help me help AIDS victims -- http://ICCF-Holland.org/// -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving