NetBSD Problem Report #44888
From www@NetBSD.org Wed Apr 20 22:42:41 2011
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
by www.NetBSD.org (Postfix) with ESMTP id E95FA63C2A1
for <gnats-bugs@gnats.NetBSD.org>; Wed, 20 Apr 2011 22:42:40 +0000 (UTC)
Message-Id: <20110420224239.C0F5F63C226@www.NetBSD.org>
Date: Wed, 20 Apr 2011 22:42:39 +0000 (UTC)
From: lwindschuh@gmail.com
Reply-To: lwindschuh@gmail.com
To: gnats-bugs@NetBSD.org
Subject: [patch] libedit read.c: fix logic
X-Send-Pr-Version: www-1.0
>Number: 44888
>Category: lib
>Synopsis: [patch] libedit read.c: fix logic
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: lib-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Wed Apr 20 22:45:00 +0000 2011
>Last-Modified: Wed Apr 20 23:10:03 +0000 2011
>Originator: Lucius Windschuh
>Release: FreeBSD 9-CURRENT
>Organization:
>Environment:
does not apply, using libedit on FreeBSD
>Description:
wpa_supplicant segfaults on my FreeBSD machine since NetBSD's libedit readline support was imported.
Digging deeper, libedit's el_push in read.c looked suspicious (here in FreeBSD's version, but with the same logic):
public void
el_push(EditLine *el, const char *str)
{
c_macro_t *ma = &el->el_chared.c_macro;
if (str != NULL && ma->level + 1 < EL_MAXMACRO) {
ma->level++;
if ((ma->macro[ma->level] = el_strdup(str)) != NULL)
return;
ma->level--;
}
term_beep(el);
term__flush();
}
It is strange to me that ma->level is not permanently incremented, only in the edge case where strdup returns NULL (which happens in my case).
So, I propose the fix blow, which not only fixes the segfaults for me, but also makes the logic consistent to me:
ma->level is incremented iff strdup() succeeds, reflecting the macro level change.
The fix to the NetBSD code is similar, the bug exists since read.c version 1.30.
>How-To-Repeat:
How to reproduce (at least on FreeBSD with enabled malloc debugging):
$ wpa_cli
> disco
> reass
(wait until more output is generated)
(press enter)
-> segfault
Backtrace from gdb:
(gdb) bt
#0 0x281f05f7 in kill () from /lib/libc.so.7
#1 0x281f0516 in raise () from /lib/libc.so.7
#2 0x281eefca in abort () from /lib/libc.so.7
#3 0x281704f3 in malloc_usable_size () from /lib/libc.so.7
#4 0x28171fda in malloc_usable_size () from /lib/libc.so.7
#5 0x28172f5a in free () from /lib/libc.so.7
#6 0x280aaf02 in read_pop (ma=0xbfbfe22c) at read.c:311
#7 0x280adb41 in el_getc (el=0x2844e300,
cp=0xbfbfe49b "ZD�\v(<�\v(\002\201D(���\017\203\n(") at read.c:345
#8 0x280b704f in el_gets (el=0x2844e300, nread=0xbfbfe4c8) at read.c:239
#9 0x280a830f in readline (p=0x805002a "> ") at /usr/src/lib/libedit/readline.c:404
#10 0x0804c4de in main (argc=Error accessing memory address 0x5: Bad address.
)
at /usr/src/usr.sbin/wpa/wpa_cli/../../../contrib/wpa//wpa_supplicant/wpa_cli.c:2038
>Fix:
Index: read.c
===================================================================
--- read.c (Revision 220736)
+++ read.c (Arbeitskopie)
@@ -217,9 +217,10 @@
if (str != NULL && ma->level + 1 < EL_MAXMACRO) {
ma->level++;
- if ((ma->macro[ma->level] = el_strdup(str)) != NULL)
+ if ((ma->macro[ma->level] = el_strdup(str)) != NULL) {
+ ma->level--;
return;
- ma->level--;
+ }
}
term_beep(el);
term__flush();
>Audit-Trail:
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: lib/44888: [patch] libedit read.c: fix logic
Date: Wed, 20 Apr 2011 18:52:33 -0400
On Apr 20, 10:45pm, lwindschuh@gmail.com (lwindschuh@gmail.com) wrote:
-- Subject: lib/44888: [patch] libedit read.c: fix logic
| It is strange to me that ma->level is not permanently incremented, only in the edge case where strdup returns NULL (which happens in my case).
It is permanently increament when strdup does not return NULL. In the error
case where strdup returns NULL it is decremented. Perhaps something else
is going on?
christos
From: Lucius Windschuh <lwindschuh@googlemail.com>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: lib/44888: [patch] libedit read.c: fix logic
Date: Thu, 21 Apr 2011 01:08:23 +0200
2011/4/21 Christos Zoulas <christos@zoulas.com>:
> =A0On Apr 20, 10:45pm, lwindschuh@gmail.com (lwindschuh@gmail.com) wrote:
> =A0-- Subject: lib/44888: [patch] libedit read.c: fix logic
>
> =A0| It is strange to me that ma->level is not permanently incremented, o=
nly in the edge case where strdup returns NULL (which happens in my case).
>
> =A0It is permanently increament when strdup does not return NULL. In the =
error
> =A0case where strdup returns NULL it is decremented. Perhaps something el=
se
> =A0is going on?
I'm sorry, you are totally right. I threw up =3D=3D NULL and !=3D NULL. :-(
Lucius
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.