NetBSD Problem Report #52348

From www@NetBSD.org  Thu Jun 29 01:46:41 2017
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 "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 76B1C7A239
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 29 Jun 2017 01:46:41 +0000 (UTC)
Message-Id: <20170629014639.8DDDA7A2B8@mollari.NetBSD.org>
Date: Thu, 29 Jun 2017 01:46:39 +0000 (UTC)
From: coypu@sdf.org
Reply-To: coypu@sdf.org
To: gnats-bugs@NetBSD.org
Subject: sh (alias.c:271) invokes undefined behaviour
X-Send-Pr-Version: www-1.0

>Number:         52348
>Category:       bin
>Synopsis:       sh (alias.c:271) invokes undefined behaviour
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kre
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jun 29 01:50:00 +0000 2017
>Closed-Date:    Sun Nov 12 03:05:21 +0000 2017
>Last-Modified:  Sun Nov 12 03:05:21 +0000 2017
>Originator:     coypu
>Release:        NetBSD 8.99.1
>Organization:
>Environment:
NetBSD loggy 8.99.1 NetBSD 8.99.1 (GENERIC) #82: Thu Jun 22 15:45:33 IDT 2017  fly@loggy:/home/fly/obj/sys/arch/amd64/compile/GENERIC amd64

>Description:
alias.c:271:15: runtime error: left shift of negative value -126

char is signed.
>How-To-Repeat:
build with asan and ubsan.
>Fix:

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->kre
Responsible-Changed-By: kre@NetBSD.org
Responsible-Changed-When: Thu, 29 Jun 2017 04:56:37 +0000
Responsible-Changed-Why:
I am looking into this PR


From: Robert Elz <kre@munnari.OZ.AU>
To: coypu@sdf.org, gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/52348: sh (alias.c:271) invokes undefined behaviour
Date: Thu, 29 Jun 2017 11:58:04 +0700

     Date:        Thu, 29 Jun 2017 01:50:00 +0000 (UTC)
     From:        coypu@sdf.org
     Message-ID:  <20170629015000.BADCE7A1FC@mollari.NetBSD.org>

   | alias.c:271:15: runtime error: left shift of negative value -126

 There is an easy (and obvious) fix for this, but I suspect that it
 might possibly indicate a deeper problem (the -126 looks like the shell's
 internal "variable reference follows" marker, rather than a character
 that should be appearing in an alias name.   If so it should be long
 gone before it gets to the code in question.

 But any char can occur in sh input, so I don't know if perhaps that
 char (0x82) was actually in the input or not.

 Can you tell me what the actual input was that causes this problem?

 That is, "problem" such as it is, << of a small negative number might not
 be a defined operation, but in practice works anywhere - and here we don't care
 what value is obtained, it is just constructing a hash value from the name.)

 kre

From: coypu@sdf.org
To: gnats-bugs@NetBSD.org
Cc: kre@NetBSD.org
Subject: Re: bin/52348: sh (alias.c:271) invokes undefined behaviour
Date: Thu, 29 Jun 2017 05:33:05 +0000

 On Thu, Jun 29, 2017 at 05:00:01AM +0000, Robert Elz wrote:
 >  Can you tell me what the actual input was that causes this problem?

 Found something else by writing down an example.
 It shows the original, too.
 I think it doesn't like the negative index use

 I'm trying to test on a real environment to get better coverage, so I'm
 doing normally ill-advised things like installing it to my real /bin.

 $ cd /usr/src/*/sh
 $ make USETOOLS=no CFLAGS="-fPIC -fsanitize=address -fsanitize=undefined" LDFLAGS="-lubsan -lasan" -j5
 $ su
 ### very dangerous step!
 ### doing 'su' again isn't possible as LD_PRELOAD is needed
 ### to run sh (and will be ignored by su).
 # make install USETOOLS=no LD_PRELOAD=/usr/lib/libasan.so

 $ export LD_PRELOAD=/usr/lib/libasan.so
 $ make distclean; make

 ...
 #    create  sh/token.h
 AWK=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbawk  MKTEMP=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbmktemp  SED=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbsed /bin/sh mktokens
 alias.c:271:15: runtime error: left shift of negative value -126
 mktokens: /usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbawk: not found
 =================================================================
 ==27210==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61900000237f at pc 0x00000040c9fd bp 0x7f7fffffdb80 sp 0x7f7fffffdb78
 READ of size 1 at 0x61900000237f thread T0
     #0 0x40c9fc in exvwarning error.c
     #1 0x40ce6c in exverror error.c
     #2 0x40d0d8 in exerror (/bin/sh+0x40d0d8)
     #3 0x41ac18 in shellexec (/bin/sh+0x41ac18)
     #4 0x418e3d in evalcommand (/bin/sh+0x418e3d)
     #5 0x40f2e0 in evaltree (/bin/sh+0x40f2e0)
     #6 0x4460f5 in cmdloop (/bin/sh+0x4460f5)
     #7 0x445c60 in main (/bin/sh+0x445c60)
     #8 0x40401a in ___start (/bin/sh+0x40401a)

 0x61900000237f is located 1 bytes to the left of 1024-byte region [0x619000002380,0x619000002780)
 freed by thread T0 here:
     #0 0x7f7ff6c15914 in __interceptor_cfree (/usr/lib/libasan.so+0x15914)
     #1 0x469827 in freestdout (/bin/sh+0x469827)
     #2 0x418519 in evalcommand (/bin/sh+0x418519)
     #3 0x40f2e0 in evaltree (/bin/sh+0x40f2e0)
     #4 0x4460f5 in cmdloop (/bin/sh+0x4460f5)
     #5 0x445c60 in main (/bin/sh+0x445c60)
     #6 0x40401a in ___start (/bin/sh+0x40401a)

 previously allocated by thread T0 here:
     #0 0x7f7ff6c15a7c in __interceptor_malloc (/usr/lib/libasan.so+0x15a7c)
     #1 0x446844 in ckmalloc (/bin/sh+0x446844)
     #2 0x468aa2 in emptyoutbuf (/bin/sh+0x468aa2)
     #3 0x40bcf3 in echocmd (/bin/sh+0x40bcf3)
     #4 0x41823b in evalcommand (/bin/sh+0x41823b)
     #5 0x40f2e0 in evaltree (/bin/sh+0x40f2e0)
     #6 0x4460f5 in cmdloop (/bin/sh+0x4460f5)
     #7 0x445c60 in main (/bin/sh+0x445c60)
     #8 0x40401a in ___start (/bin/sh+0x40401a)

 SUMMARY: AddressSanitizer: heap-buffer-overflow error.c:0 exvwarning
 Shadow bytes around the buggy address:
   0x0c327fff8410: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0c327fff8420: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0c327fff8430: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0c327fff8440: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0c327fff8450: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
 =>0x0c327fff8460: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
   0x0c327fff8470: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
   0x0c327fff8480: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
   0x0c327fff8490: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
   0x0c327fff84a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
   0x0c327fff84b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
 Shadow byte legend (one shadow byte represents 8 application bytes):
   Addressable:           00
   Partially addressable: 01 02 03 04 05 06 07 
   Heap left redzone:       fa
   Heap right redzone:      fb
   Freed heap region:       fd
   Stack left redzone:      f1
   Stack mid redzone:       f2
   Stack right redzone:     f3
   Stack partial redzone:   f4
   Stack after return:      f5
   Stack use after scope:   f8
   Global redzone:          f9
   Global init order:       f6
   Poisoned by user:        f7
   Container overflow:      fc
   Array cookie:            ac
   Intra object redzone:    bb
   ASan internal:           fe
 ==27210==ABORTING
 ==25994==AddressSanitizer: while reporting a bug found another one. Ignoring.
 ==13051==AddressSanitizer: while reporting a bug found another one. Ignoring.
 ==28262==AddressSanitizer: while reporting a bug found another one. Ignoring.
 #    create  sh/nodenames.h
 AWK=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbawk  MKTEMP=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbmktemp  SED=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbsed /bin/sh mknodenames.sh nodes.h > nodenames.h
 #    create  sh/optinit.h
 AWK=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbawk  MKTEMP=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbmktemp  SED=/usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbsed /bin/sh mkoptions.sh option.list optinit.h /usr/src/bin/sh
 alias.c:271:15: runtime error: left shift of negative value -126
 mkoptions.sh: /usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbmktemp: not found
 mkoptions.sh: /usr/src/obj/tooldir.NetBSD-8.99.1-amd64/bin/nbmktemp: not found
 mkoptions.sh: cannot create : directory nonexistent
 *** Error code 2

From: coypu@sdf.org
To: Robert Elz <kre@munnari.OZ.AU>
Cc: gnats-bugs@NetBSD.org
Subject: Re: bin/52348: sh (alias.c:271) invokes undefined behaviour
Date: Thu, 29 Jun 2017 05:57:39 +0000

 It's not very critical at all, I wouldn't even report it to another
 project. maybe I will focus on out of bound writes, if I can get to
 them.

From: Robert Elz <kre@munnari.OZ.AU>
To: coypu@sdf.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: bin/52348: sh (alias.c:271) invokes undefined behaviour
Date: Thu, 29 Jun 2017 13:12:11 +0700

     Date:        Thu, 29 Jun 2017 05:33:05 +0000
     From:        coypu@sdf.org
     Message-ID:  <20170629053305.GA22727@SDF.ORG>

   | Found something else by writing down an example.

 I will see what I can work out about that one (aside from why the tools awk
 does not exist in your system, and yet it is being used - or wants to
 be, same for nbmktemp - those I can't help with....)

 However, debugging sh memory management is hit and miss at the best of
 times, it will be pure fluke if I can find anything from that asan output.
 I would need you to be running a DEBUG shell with every possible debug
 flag turned on when it happened (and possibly gdb as well) to have any
 real expectation of finding that one.  At least I know the situation in
 which it was observed, which helps a little.

   | It shows the original, too.

 Not really.   It shows the error message.   But it tells me where you
 should look, as there are precisely zero uses of anything related to
 aliases while building sh (except compiling the code to process them.)

 To be in the alias code, something must be making it happen, and the
 only candidate (I can see) is the file which is your $ENV

 That it happened twice. later running a different script, reinforces that.

 So, please send me any lines from $ENV that have anything to do with aliases
 (or just the whole thing) - send via private e-mail, it does not need to
 go into the PR!

   | I think it doesn't like the negative index use

 There is no negative index.   I understand what asan is saying, it
 is being over petty in this particular case, though it is correct, the
 code in alias.c is wrong, and I will fix it (it isn't even a very good
 hash function, not that it needs to be for aliases, most people don't
 have very many.)

 What I want to do is find out how the situation which allowed the bug
 to be discovered arose - if it is just "unusual" input, then I just fix
 (or perhaps change is a better word) the code in alias.c and all will
 be fine.   On the other hand, there might be some other bug.   Only seeing
 the actual line that the sh is executing when the error is produced
 will tell me that -- I don't need to be running asan I don't even really,
 I hope, need to run a sh to process your $ENV (or lines from it), all I
 should need is to see it.

 kre


From: Robert Elz <kre@munnari.OZ.AU>
To: coypu@sdf.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: bin/52348: sh (alias.c:271) invokes undefined behaviour
Date: Thu, 29 Jun 2017 13:19:48 +0700

 It is not important if it is just being caused by an "unusual" (to me)
 alias name - otherwise there might be some much more significant bug.

 I would like to work out which it is, then I will fix the alias.c code.

 kre

From: coypu@sdf.org
To: Robert Elz <kre@munnari.OZ.AU>
Cc: gnats-bugs@NetBSD.org
Subject: Re: bin/52348: sh (alias.c:271) invokes undefined behaviour
Date: Thu, 29 Jun 2017 06:25:30 +0000

 I could get useful info by doing this:

 #include <assert.h>
 STATIC struct alias **
 hashalias(const char *p)
 {
 	unsigned int hashval;

 	assert(*p > -5);
 	hashval = *p << 4;
 	while (*p)
 		hashval+= *p++;
 	return &atab[hashval % ATABSIZE];
 }

 But instead it gives me in these cases:
 [1]   Abort trap              AWK=/usr/src/obj...
 *** Error code 134

 and no coredump.

From: Robert Elz <kre@munnari.OZ.AU>
To: coypu@sdf.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: bin/52348: sh (alias.c:271) invokes undefined behaviour
Date: Thu, 29 Jun 2017 14:04:46 +0700

     Date:        Thu, 29 Jun 2017 06:25:30 +0000
     From:        coypu@sdf.org
     Message-ID:  <20170629062530.GC22727@SDF.ORG>

   | But instead it gives me in these cases:
   | [1]   Abort trap              AWK=/usr/src/obj...
   | *** Error code 134
   | 
   | and no coredump.

 That usually means that for some reason there's no permission to
 write the core file.   Not that it is really likely to have helped much.

 More useful info (if you really insist on debugging this this way)
 would be

 	if (*p < 0) out2fmt("Weird alias: <<%s>>\n", p);

 and then run the shell with stderr directed to a file, so we see
 exactly what bytes are printed, not to a terminal with cut & paste later).

 You could probably add, to your $ENV file

 	exec 7>&2 2>/tmp/sh-alias-debug

 (pick your own file name) right at the top, and then

 	exec 2>&7 7>&-

 at the end, to allow just the $ENV processing to have stderr redirected.


 But still, the most useful thing you can so is send me your $ENV (or
 at least any lines in it which define, or use aliases, and any syntax,
 like loops, fuctions, etc) that enclose them.   That is what will
 provide the best info.

 kre

State-Changed-From-To: open->analyzed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Mon, 17 Jul 2017 02:08:36 +0000
State-Changed-Why:
I have fixes for both of the issues shown in this PR.
Neither is important (that is, neither would have any
adverse effect whatever).   The fixes will be committed
sometime, but there is no hurry.

I would still like to discover what triggered detection of the
alias issue, just to make sure there is not some other bug
lurking that could be more serious (if it is just an alias using
a non-ascii character, then there is no issue, and all is OK,
if not, something weird must be happening.)


From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52348 CVS commit: src/bin/sh
Date: Mon, 24 Jul 2017 12:34:46 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Mon Jul 24 12:34:46 UTC 2017

 Modified Files:
 	src/bin/sh: alias.c

 Log Message:
 PR bin/52348

 Silence nuisance testing environments - avoid << of a negative number
 (a signed char -- in a hash function, the result is irrelevant, as long
 as it is repeatable).


 To generate a diff of this commit:
 cvs rdiff -u -r1.15 -r1.16 src/bin/sh/alias.c

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

From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52348 CVS commit: src/bin/sh
Date: Mon, 24 Jul 2017 12:35:12 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Mon Jul 24 12:35:12 UTC 2017

 Modified Files:
 	src/bin/sh: error.c

 Log Message:
 PR bin/52348

 Avoid a reference after free (detected by asan) - harmless here, but
 easy to fix.


 To generate a diff of this commit:
 cvs rdiff -u -r1.40 -r1.41 src/bin/sh/error.c

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

State-Changed-From-To: analyzed->feedback
State-Changed-By: kre@NetBSD.org
State-Changed-When: Tue, 25 Jul 2017 02:11:34 +0000
State-Changed-Why:
I believe both of the issues detected here are fixed (in HEAD) now.
Neither is a serious problem (ie: neither will ever affect anything.)

Can you test again and confirm?

I'd still like to know what alias triggered the alias issue.


State-Changed-From-To: feedback->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Sun, 12 Nov 2017 03:05:21 +0000
State-Changed-Why:
Believed fixed, and no longer able to be tested.


>Unformatted:

Home	PR Database Search