NetBSD Problem Report #54272

From www@netbsd.org  Tue Jun  4 20:25:26 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 008507A15D
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  4 Jun 2019 20:25:25 +0000 (UTC)
Message-Id: <20190604202524.D581B7A1D2@mollari.NetBSD.org>
Date: Tue,  4 Jun 2019 20:25:24 +0000 (UTC)
From: mitchell.felton@wdc.com
Reply-To: mitchell.felton@wdc.com
To: gnats-bugs@NetBSD.org
Subject: libedit segfaults on Linux aarch64 build
X-Send-Pr-Version: www-1.0

>Number:         54272
>Category:       lib
>Synopsis:       libedit segfaults on Linux aarch64 build
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 04 20:30:00 +0000 2019
>Last-Modified:  Fri Jun 07 15:00:01 +0000 2019
>Originator:     Mitch Felton
>Release:        libedit-20190324-3.1
>Organization:
Western Digital
>Environment:
Linux dhcp-10-202-62-242 4.15.0-50-generic #54-Ubuntu SMP Tue May 7 05:57:03 UTC 2019 aarch64 aarch64 aarch64 GNU/Linux
>Description:
I encounter a segfault when using an aarch64 compiled version of libedit version 3.1 (2019-03-24).
>How-To-Repeat:

>Fix:
There are 2 fixes in src/terminal.c in the terminal_set() function.
- Initialize 'buf' to zeros before using it
- pass 'area' into terminal_alloc() correctly
A patch for these changes is shown below. These 2 changes fix the segfault and libedit functions correctly for me with them.

--- a/src/terminal.c
+++ b/src/terminal.c
@@ -848,6 +848,8 @@ terminal_set(EditLine *el, const char *term)
 	sigset_t oset, nset;
 	int lins, cols;

+	(void) memset(buf, 0, TC_BUFSIZE);
+
 	(void) sigemptyset(&nset);
 	(void) sigaddset(&nset, SIGWINCH);
 	(void) sigprocmask(SIG_BLOCK, &nset, &oset);
@@ -898,7 +900,7 @@ terminal_set(EditLine *el, const char *term)
 		for (t = tstr; t->name != NULL; t++) {
 			/* XXX: some systems' tgetstr needs non const */
 			terminal_alloc(el, t, tgetstr(strchr(t->name, *t->name),
-			    &area));
+			    area));
 		}
 	}

This fix is offered under the BSD license by Mitch Felton, an employee of Western Digital.

>Audit-Trail:
From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org,
 gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org
Subject: Re: lib/54272: libedit segfaults on Linux aarch64 build
Date: Tue, 4 Jun 2019 20:51:50 -0400

 This does not seem right:

 1. The "area" argument is not an argument to terminal_alloc(), but to =
 tgetstr() which takes as the third argument a char **. In fact you =
 should get a warning if you remove the &.
 2. The initialization is extraneous since the area argument is only =
 supposed to be used as scratch space in tgetstr().

 What might be happening, is that tgetstr() writes to the id argument on =
 linux?

 christos=

From: Mitch Felton <Mitchell.Felton@wdc.com>
To: "gnats-bugs@netbsd.org" <gnats-bugs@netbsd.org>,
	"lib-bug-people@netbsd.org" <lib-bug-people@netbsd.org>,
	"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>, "netbsd-bugs@netbsd.org"
	<netbsd-bugs@netbsd.org>
Cc: 
Subject: RE: lib/54272: libedit segfaults on Linux aarch64 build
Date: Fri, 7 Jun 2019 14:40:02 +0000

 I spent some more time debugging this today and you are correct that my fix=
  is wrong. It looks like the tgetstr() call that you mentioned is returning=
  a pointer to invalid memory. Here's the signature of tgetstr():
     char *tgetstr(char *id, char **area);
 The difference between a good run on my x86_64 system and a bad run on my a=
 arch64 system seems to be that the return pointer has the upper 16 bits all=
  set (0xffff000000000000). For example, it returns 0xfffffffff89b7fa7 inste=
 ad of 0xfffff89b7fa7 (which is a pointer to the area buffer that it updated=
 ). So it appears that this is in fact a problem somewhere in tgetstr() and =
 not with libedit.

 Mitch

 -----Original Message-----
 From: Christos Zoulas <christos@zoulas.com>=20
 Sent: Tuesday, June 4, 2019 7:55 PM
 To: lib-bug-people@netbsd.org; gnats-admin@netbsd.org; netbsd-bugs@netbsd.o=
 rg; Mitch Felton <Mitchell.Felton@wdc.com>
 Subject: Re: lib/54272: libedit segfaults on Linux aarch64 build

 The following reply was made to PR lib/54272; it has been noted by GNATS.

 From: Christos Zoulas <christos@zoulas.com>
 To: gnats-bugs@netbsd.org
 Cc: lib-bug-people@netbsd.org,
  gnats-admin@netbsd.org,
  netbsd-bugs@netbsd.org
 Subject: Re: lib/54272: libedit segfaults on Linux aarch64 build
 Date: Tue, 4 Jun 2019 20:51:50 -0400

  This does not seem right:
 =20
  1. The "area" argument is not an argument to terminal_alloc(), but to =3D
  tgetstr() which takes as the third argument a char **. In fact you =3D  sh=
 ould get a warning if you remove the &.
  2. The initialization is extraneous since the area argument is only =3D  s=
 upposed to be used as scratch space in tgetstr().
 =20
  What might be happening, is that tgetstr() writes to the id argument on =
 =3D  linux?
 =20
  christos=3D
 =20

From: christos@zoulas.com (Christos Zoulas)
To: Mitch Felton <Mitchell.Felton@wdc.com>, 
	"gnats-bugs@netbsd.org" <gnats-bugs@netbsd.org>, 
	"lib-bug-people@netbsd.org" <lib-bug-people@netbsd.org>, 
	"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>, 
	"netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Cc: 
Subject: RE: lib/54272: libedit segfaults on Linux aarch64 build
Date: Fri, 7 Jun 2019 10:57:52 -0400

 On Jun 7,  2:40pm, Mitchell.Felton@wdc.com (Mitch Felton) wrote:
 -- Subject: RE: lib/54272: libedit segfaults on Linux aarch64 build

 | I spent some more time debugging this today and you are correct that my fix=
 |  is wrong. It looks like the tgetstr() call that you mentioned is returning=
 |  a pointer to invalid memory. Here's the signature of tgetstr():
 |     char *tgetstr(char *id, char **area);
 | The difference between a good run on my x86_64 system and a bad run on my a=
 | arch64 system seems to be that the return pointer has the upper 16 bits all=
 |  set (0xffff000000000000). For example, it returns 0xfffffffff89b7fa7 inste=
 | ad of 0xfffff89b7fa7 (which is a pointer to the area buffer that it updated=
 | ). So it appears that this is in fact a problem somewhere in tgetstr() and =
 | not with libedit.

 Thanks Mitch, I would suggest running this with valgrind or turning on
 the address sanitizer on gcc... Although you might need compile ncurses
 with it, so valgrind is probably easier.

 christos

Home	PR Database Search