NetBSD Problem Report #42412

From dholland@eecs.harvard.edu  Fri Dec  4 21:48:45 2009
Return-Path: <dholland@eecs.harvard.edu>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 922C663C3AC
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  4 Dec 2009 21:48:45 +0000 (UTC)
Message-Id: <20091204214753.A94C3FA8D@tanaqui.eecs.harvard.edu>
Date: Fri,  4 Dec 2009 16:47:53 -0500 (EST)
From: dholland@eecs.harvard.edu
Reply-To: dholland@eecs.harvard.edu
To: gnats-bugs@gnats.NetBSD.org
Subject: atc(6) segfaults on invalid $TERM
X-Send-Pr-Version: 3.95

>Number:         42412
>Category:       lib
>Synopsis:       atc(6) segfaults on invalid $TERM
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Dec 04 21:50:03 +0000 2009
>Closed-Date:    Thu Jun 03 05:38:28 +0000 2010
>Last-Modified:  Thu Jun 10 05:25:02 +0000 2010
>Originator:     David A. Holland
>Release:        NetBSD 5.99.15 (~20090831)
>Organization:
>Environment:
System: NetBSD tanaqui 5.99.15 NetBSD 5.99.15 (TANAQUI) #29: Thu Sep 3 18:23:41 EDT 2009 dholland@tanaqui:/usr/src/sys/arch/i386/compile/TANAQUI i386
Architecture: i386
Machine: i386
>Description:

% env TERM=fubar /usr/games/atc
Segmentation fault
% 

>How-To-Repeat:

as above

>Fix:

probably trivial
(this PR is so I don't forget)

>Release-Note:

>Audit-Trail:
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/42412: atc(6) segfaults on invalid $TERM
Date: Sun, 6 Dec 2009 06:58:06 +0000

 On Fri, Dec 04, 2009 at 09:50:03PM +0000, dholland@eecs.harvard.edu wrote:
  > % env TERM=fubar /usr/games/atc
  > Segmentation fault
  > % 

 Starting program: /usr/obj/games/atc/atc 

 Program received signal SIGSEGV, Segmentation fault.
 0xbbbc5512 in t_goto (info=0x0, 
     CM=0x20203020 <Address 0x20203020 out of bounds>, destcol=0, destline=0, 
     buffer=0xbfbfea84 "\b.......|" , limit=63)
     at /usr/src/lib/libterm/tgoto.c:128
 128             while ((c = *cp++) != '\0') {
 (gdb) where
 #0  0xbbbc5512 in t_goto (info=0x0, 
     CM=0x20203020 <Address 0x20203020 out of bounds>, destcol=0, destline=0, 
     buffer=0xbfbfea84 "\b.......|", limit=63)
     at /usr/src/lib/libterm/tgoto.c:128
 #1  0xbbbb6bdc in _cursesi_setterm (type=0xbfbffea2 "fubar", screen=0xbb915000)
     at /usr/src/lib/libcurses/setterm.c:196
 #2  0xbbbb495f in newterm (type=0xbfbffea2 "fubar", outfd=0x8053b20, 
     infd=0x8053ac8) at /usr/src/lib/libcurses/screen.c:154
 #3  0xbbbae495 in initscr () at /usr/src/lib/libcurses/initscr.c:68
 #4  0x0804add9 in init_gr () at /usr/src/games/atc/graphics.c:120
 #5  0x0804f176 in main (argc=1, argv=0xbfbfec84)
     at /usr/src/games/atc/main.c:144
 (gdb) p cp
 $2 = 0x20203020 <Address 0x20203020 out of bounds>
 (gdb) 

 This appears to come from screen->tc_cm in the caller. It's not
 immediately clear to me what's supposed to be happening for undefined
 terminal types, but it's clearly wrong.

 -- 
 David A. Holland
 dholland@netbsd.org

Responsible-Changed-From-To: bin-bug-people->lib-bug-people
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Sun, 06 Dec 2009 07:01:24 +0000
Responsible-Changed-Why:
Problem is in curses.


From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/42412 (atc(6) segfaults on invalid $TERM)
Date: Sun, 6 Dec 2009 10:36:10 +0000

 On Sun, Dec 06, 2009 at 07:01:25AM +0000, dholland@NetBSD.org wrote:
  > Problem is in curses.

 Based on some digging and discussion in chat:

 Index: screen.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libcurses/screen.c,v
 retrieving revision 1.21
 diff -u -p -r1.21 screen.c
 --- screen.c	8 Dec 2007 18:38:11 -0000	1.21
 +++ screen.c	6 Dec 2009 10:30:24 -0000
 @@ -117,7 +117,7 @@ newterm(char *type, FILE *outfd, FILE *i
  	if ((type == NULL) && (sp = getenv("TERM")) == NULL)
  		return NULL;

 -	if ((new_screen = (SCREEN *) malloc(sizeof(SCREEN))) == NULL)
 +	if ((new_screen = calloc(1, sizeof(SCREEN))) == NULL)
  		return NULL;

  #ifdef DEBUG


 The problem is that logic inside _cursesi_setterm and t_goto is
 assuming that the CM field will be null if the terminal type is
 unknown, but it's getting trash instead. Something fairly recently
 (perhaps time_t?) reshuffled the malloc behavior and the resulting
 memory layout, so malloc is now handing back memory that's been used
 rather than still-zero memory. This is why it only broke recently even
 though this logic in curses hasn't been touched in years.

 Judging by how much of the SCREEN contains trash in the failure case,
 and based on the presumption that there may be other similar logic
 elsewhere, it seems like unconditionally clearing the thing is the
 best approach. (Especially for the stable branches.)

 -- 
 David A. Holland
 dholland@netbsd.org

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Thu, 03 Jun 2010 05:38:28 +0000
State-Changed-Why:
The crash went away with the terminfo curses.


From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/42412 CVS commit: src/lib/libcurses
Date: Thu, 10 Jun 2010 05:24:56 +0000

 Module Name:	src
 Committed By:	dholland
 Date:		Thu Jun 10 05:24:56 UTC 2010

 Modified Files:
 	src/lib/libcurses: screen.c

 Log Message:
 After consultation with jdc@, apply the patch from PR 42412 even though
 the crash went away -- it's not clear that it went away for any reason
 besides by accident.

 The change is: use calloc instead of malloc for allocating SCREENs.


 To generate a diff of this commit:
 cvs rdiff -u -r1.22 -r1.23 src/lib/libcurses/screen.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search