On Friday, August 14, 2015 at 2:38:57 PM UTC-4, Bram Moolenaar wrote: > Raymond Ko wrote: > > > On Wednesday, August 12, 2015 at 4:04:57 PM UTC-4, Dominique Pelle wrote: > > > Hi > > > > > > Here is another bug discovered by afl-fuzz which is > > > ruthless at finding bugs. The following command > > > crashes vim-7.4.823 (and older): > > > > > > $ vim -u NONE -c ow -c 'sy keyword x c' > > > Vim: Caught deadly signal SEGV > > > Vim: Finished. > > > Segmentation fault (core dumped) > > > > > > $ cgdb --args ./vim -u NONE -c ow -c 'sy keyword x c' > > > > > > 153│ idx = (unsigned)(hash & ht->ht_mask); > > > 154│ hi = &ht->ht_array[idx]; > > > 155│ > > > 156├> if (hi->hi_key == NULL) > > > 157│ return hi; > > > > > > (gdb) p hi > > > $1 = (hashitem_T *) 0x0 > > > > > > (gdb) p idx > > > $2 = 0 > > > > > > (gdb) p hash > > > $3 = 99 > > > > > > (gdb) p ht->ht_mask > > > $4 = 0 > > > > > > (gdb) bt > > > #0 0x00000000004e2b5d in hash_lookup (ht=0x965a00, key=0x965f48 "c", > > > hash=99) at hashtab.c:156 > > > #1 0x000000000061ca44 in add_keyword (name=0x965f00 "c", id=46, > > > flags=0, cont_in_list=0x0, next_list=0x0, conceal_char=0) at > > > syntax.c:4458 > > > #2 0x00000000006174f5 in syn_cmd_keyword (eap=0x7fffffffd428, > > > syncing=0) at syntax.c:4868 > > > #3 0x000000000060f0b5 in ex_syntax (eap=0x7fffffffd428) at syntax.c:6296 > > > #4 0x000000000049053b in do_one_cmd (cmdlinep=0x7fffffffdb88, > > > sourcing=1, cstack=0x7fffffffd6d0, fgetline=0x0, cookie=0x0) at > > > ex_docmd.c:2941 > > > #5 0x000000000048bdfe in do_cmdline (cmdline=0x7fffffffe2a0 "sy > > > keyword x c", fgetline=0x0, cookie=0x0, flags=11) at ex_docmd.c:1133 > > > #6 0x000000000048cca6 in do_cmdline_cmd (cmd=0x7fffffffe2a0 "sy > > > keyword x c") at ex_docmd.c:738 > > > #7 0x00000000006777e8 in exe_commands (parmp=0x7fffffffdc78) at > > > main.c:2926 > > > #8 0x00000000006742fd in main (argc=7, argv=0x7fffffffdeb8) at main.c:961 > > > > > > Valgrind or asan don't give more info. > > > Sorry no patch. I'm not sure how to fix it. > > > > > > Regards > > > Dominique > > > > This struck me as odd as I was tweaking VIM's hash table to try and > > lower collision rate in my own minifork, and I thought: there can't > > possibly be a bug in the hash table! > > > > Anyways after some investigation, the cause of the crash is that > > :ownsyntax creates it's own synblock_T but does not properly > > initialize it, i.e. memset to 0 is wrong for some structures. For most > > things, it is okay, except for the spellcheck structures, but that is > > marked as TODO and set to values that disable it. Looking at GDB print > > of structures, the only other structures which seem essential to be > > initialized is is the two hash tables b_keywtab && b_keywtab_ic. A > > simple hash_init on these two tables fixes the crash reported as shown > > in the patch below. > > Thanks for creating a fix! > > > A proper solution would probably be to completely deep copy the entire > > structure from the buffer, and add a comment in the structs.h to say > > to update that init function if you change it, but maybe there is no > > one that actually uses :ownsyntax and spell check together so just add > > to TODO list if not already? > > Can you think of another way this would currently break? Or is this > just to avoid problems in the future. > > -- > GUEST: He's killed the best man! > SECOND GUEST: (holding a limp WOMAN) He's killed my auntie. > FATHER: No, please! This is supposed to be a happy occasion! Let's > not bicker and argue about who killed who ... > "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/ \\\ > \\\ an exciting new programming language -- http://www.Zimbu.org /// > \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
It is just to avoid problems in the future. I haven't seen anything that indicates it will break in a tricky fashion like this. Maybe the garray_T as that is an embedded struct instead of a pointer, compared to a NULL pointer, which will be obvious. I think it should have ga_init() called on them, but then again it seem that even a buffer's own synblock_T isn't initialized that way. It seems like it is not worth it unless garray_T is being redesigned to need non-NULL values as part of intialization. -- -- 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
