zsh-workers
 help / color / mirror / Atom feed
* segfault in bindkey -d
@ 2021-08-22 22:01 Roman Neuhauser
  2021-09-06 10:19 ` Jun T
  0 siblings, 1 reply; 2+ messages in thread
From: Roman Neuhauser @ 2021-08-22 22:01 UTC (permalink / raw)
  To: zsh-workers

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: segfault in bindkey -d
  2021-08-22 22:01 segfault in bindkey -d Roman Neuhauser
@ 2021-09-06 10:19 ` Jun T
  0 siblings, 0 replies; 2+ messages in thread
From: Jun T @ 2021-09-06 10:19 UTC (permalink / raw)
  To: zsh-workers



> 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





^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-09-06 10:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-22 22:01 segfault in bindkey -d Roman Neuhauser
2021-09-06 10:19 ` Jun T

zsh-workers

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.vuxu.org/zsh-workers

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 zsh-workers zsh-workers/ https://inbox.vuxu.org/zsh-workers \
		zsh-workers@zsh.org
	public-inbox-index zsh-workers

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/zsh/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git