segfault in bindkey -d

segfault in bindkey -d

[Roman Neuhauser]

hello there,

this reliably crashes the shell, both 5.8.0 and zsh-5.8-462-g765bc14:

bindkey -N a; bindkey -N b; bindkey -N c; bindkey -N d; bindkey -N e; bindkey -d

-- 
roman

Re: segfault in bindkey -d

[Jun T]

> 2021/08/23 7:01, Roman Neuhauser <neuhauser@sigpipe.cz> wrote:

Thanks for the report, roman.

> this reliably crashes the shell, both 5.8.0 and zsh-5.8-462-g765bc14:
> 
> bindkey -N a; bindkey -N b; bindkey -N c; bindkey -N d; bindkey -N e; bindkey -d

If five keymaps 'a' .. 'e' are added, total number of keymaps becomes 14 = 2*7
(7 is the initial size of the hash table 'keymapnamtab'), and expandhashtable()
is called for keymapnamtab. As a result order of the keymaps in the table changes.

This should not cause any problem, of cause. But with the current code, when
removing all the keymaps by 'bindkey -d', we get coredump if 'emacs' is removed
before 'main'.

'bindkey -d' calls:
	keymapnamtab->emptytable() = emptyhashtable()
   	resizehashtable()
	ht->freenode() (hashtalbe.c:496) = freekeymapnamnode() (zle_keymap.c)
	unrefkeymap_by_name().

When unrefkeymap_by_name() is called for the 'emacs' keymap, scanhashtable() is
called because the reference count of the keymap (return value of unrefkeymap(km))
is nonzero and 'main' is the primary name for the keymap.
But calling scanhashtable() for a table that is currently being erased can cause
a coredump (at line 400 in hashtable.c).

Since we need not bother correctly setting km->primary when erasing all the
keymaps, a possible fix is to create a function to erase keymapnamtab and use it
for keymapnamtab->emptytable instead of the general function emptyhashtable().

Also added a test in X03zlebindkey.ztst (assuming that adding five keymaps will
call expandhashtable() in all the future versions of zsh...). 


diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c
index 2389ab754..d90838f03 100644
--- a/Src/Zle/zle_keymap.c
+++ b/Src/Zle/zle_keymap.c
@@ -155,7 +155,7 @@ createkeymapnamtab(void)
     keymapnamtab = newhashtable(7, "keymapnamtab", NULL);
 
     keymapnamtab->hash        = hasher;
-    keymapnamtab->emptytable  = emptyhashtable;
+    keymapnamtab->emptytable  = emptykeymapnamtab;
     keymapnamtab->filltable   = NULL;
     keymapnamtab->cmpnodes    = strcmp;
     keymapnamtab->addnode     = addhashnode;
@@ -178,6 +178,26 @@ makekeymapnamnode(Keymap keymap)
     return kmn;
 }
 
+/**/
+static void
+emptykeymapnamtab(HashTable ht)
+{
+    struct hashnode *hn, *hp;
+    int i;
+
+    for (i = 0; i < ht->hsize; i++) {
+	for (hn = ht->nodes[i]; hn;) {
+	    KeymapName kmn = (KeymapName) hn;
+	    hp = hn->next;
+	    zsfree(kmn->nam);
+	    unrefkeymap(kmn->keymap);
+	    zfree(kmn, sizeof(*kmn));
+	    hn = hp;
+	}
+	ht->nodes[i] = NULL;
+    }
+    ht->ct = 0;
+}
 
 /*
  * Reference a keymap from a keymapname.
diff --git a/Test/X03zlebindkey.ztst b/Test/X03zlebindkey.ztst
index d643b1ec9..3e299a337 100644
--- a/Test/X03zlebindkey.ztst
+++ b/Test/X03zlebindkey.ztst
@@ -141,3 +141,18 @@
 >CURSOR: 18
 >BUFFER: echo $(( ##x ) ##x ) y
 >CURSOR: 22
+
+  bindkey -d
+  for name in a b c d e; bindkey -N $name
+  bindkey -d
+  bindkey -l
+0:delete all keymaps after expanding keymapnamtab
+>.safe
+>command
+>emacs
+>isearch
+>main
+>vicmd
+>viins
+>viopp
+>visual