NetBSD Problem Report #52986

From martin@duskware.de  Wed Feb  7 15:50:29 2018
Return-Path: <martin@duskware.de>
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 D5D4A7A195
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  7 Feb 2018 15:50:28 +0000 (UTC)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: netpgpverify broken on sparc64
X-Send-Pr-Version: 3.95

>Number:         52986
>Category:       bin
>Synopsis:       netpgpverify broken on sparc64
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    agc
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Feb 07 15:55:00 +0000 2018
>Closed-Date:    Fri Mar 09 14:45:36 +0000 2018
>Last-Modified:  Fri Mar 09 14:45:36 +0000 2018
>Originator:     Martin Husemann
>Release:        NetBSD 8.99.12
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD thirdstage.duskware.de 8.99.12 NetBSD 8.99.12 (MODULAR) #59: Tue Jan 30 13:52:49 CET 2018 martin@thirdstage.duskware.de:/usr/src/sys/arch/sparc64/compile/MODULAR sparc64
Architecture: sparc64
Machine: sparc64
>Description:

Recent changes (to openssl or netpgp) did break the atf tests for netpgpverify
on sparc64.

>How-To-Repeat:

Extract the dsa-pubring.gpg and in2.asc files from 
/usr/tests/usr.bin/netpgpverify/t_netpgpverify by uudecoding the respective
chunks. Then:

gdb netpgpverify
(gdb) break digest_init
(gdb) run -k dsa-pubring.gpg in2.asc
(gdb) cont
(gdb) cont
(gdb) cont
(gdb) p hashalg
$1 = 0

and watch:

109             switch(hash->alg = hashalg) {
110             case MD5_HASH_ALG:
...
145             default:
146                     printf("hash_any: bad algorithm\n");
147                     return 0;
148             }

... hit the default case and fail.

>Fix:
n/a

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->agc
Responsible-Changed-By: agc@NetBSD.org
Responsible-Changed-When: Wed, 07 Feb 2018 18:04:13 +0000
Responsible-Changed-Why:
This is mine


From: Martin Husemann <martin@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/52986: netpgpverify broken on sparc64
Date: Sun, 11 Feb 2018 15:35:50 +0000

 Something overwrites memory where it shouldn't:

 Setting a breakpoint on match_sig, and then on get_ref
 shows two hits to get_ref, both with the same "ref" argument,
 but memory changing between both calls (one for binary signature,
 second try for ascii armored):

 first get_ref:

 (gdb) p *ref
 $22 = {vp = 0x40802060, offset = 2, mem = 3}
 (gdb) p *pgp
 $23 = {pktsc = 10, pktsvsize = 5000, pktss = 0x40900000, primariesc = 1,
   primariesvsize = 10, primariess = 0x4080c000, areasc = 4, areasvsize = 10,
   areass = 0x40804060, datastartsc = 1, datastartsvsize = 10,
   datastartss = 0x4080a250, signaturesc = 2, signaturesvsize = 10,
   signaturess = 0x4081e800, signed_useridsc = 1, signed_useridsvsize = 10,
   signed_useridss = 0x40804240, signed_userattrsc = 0,
   signed_userattrsvsize = 0, signed_userattrss = 0x0, signed_subkeysc = 1,
   signed_subkeysvsize = 10, signed_subkeyss = 0x40827000, subpktsc = 12,
   subpktsvsize = 30, subpktss = 0x40816400, pkt = 10,
   op = 0x149110 "signature", ssh = 0}
 (gdb) p *mem
 $24 = {size = 96, cc = 96, mem = 0xffffffffffffad08 "\210^\004\001\021\b",
   fp = 0x0, dealloc = 0 '\000', allowed = 0x1492b0 "\002\004\b\v"}

 here mem->mem is ok and we extract the proper hashalg a few bytes into that.
 But on second call:

 second get_ref:
 $25 = {vp = 0x40802060, offset = 2, mem = 3}
 (gdb) p *pgp
 $26 = {pktsc = 10, pktsvsize = 5000, pktss = 0x40900000, primariesc = 1,
   primariesvsize = 10, primariess = 0x4080c000, areasc = 4, areasvsize = 10,
   areass = 0x40804060, datastartsc = 1, datastartsvsize = 10,
   datastartss = 0x4080a250, signaturesc = 2, signaturesvsize = 10,
   signaturess = 0x4081e800, signed_useridsc = 1, signed_useridsvsize = 10,
   signed_useridss = 0x40804240, signed_userattrsc = 0,
   signed_userattrsvsize = 0, signed_userattrss = 0x0, signed_subkeysc = 1,
   signed_subkeysvsize = 10, signed_subkeyss = 0x40827000, subpktsc = 12,
   subpktsvsize = 30, subpktss = 0x40816400, pkt = 10,
   op = 0x149110 "signature", ssh = 0}
 (gdb) p *mem
 $27 = {size = 96, cc = 96, mem = 0xffffffffffffad08 "", fp = 0x0,
   dealloc = 0 '\000', allowed = 0x1492b0 "\002\004\b\v"}

 and we get hashalg == 0.

 Martin

From: Martin Husemann <martin@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: agc@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
	martin@NetBSD.org
Subject: Re: bin/52986: netpgpverify broken on sparc64
Date: Sun, 11 Feb 2018 16:18:03 +0000

 On Sun, Feb 11, 2018 at 03:40:01PM +0000, Martin Husemann wrote:
 >  (gdb) p *mem
 >  $24 = {size = 96, cc = 96, mem = 0xffffffffffffad08 "\210^\004\001\021\b",
 >    fp = 0x0, dealloc = 0 '\000', allowed = 0x1492b0 "\002\004\b\v"}
 >  
 >  here mem->mem is ok and we extract the proper hashalg a few bytes into that.

 But note that this memory is on the stack (or did I miscount the f's?) - I bet
 that is not planned to be so.

 Martin

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, agc@NetBSD.org, gnats-admin@netbsd.org, 
	netbsd-bugs@netbsd.org, martin@NetBSD.org
Cc: 
Subject: Re: bin/52986: netpgpverify broken on sparc64
Date: Sun, 11 Feb 2018 11:33:28 -0500

 On Feb 11,  4:20pm, martin@netbsd.org (Martin Husemann) wrote:
 -- Subject: Re: bin/52986: netpgpverify broken on sparc64

 | The following reply was made to PR bin/52986; it has been noted by GNATS.
 | 
 | From: Martin Husemann <martin@netbsd.org>
 | To: gnats-bugs@NetBSD.org
 | Cc: agc@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
 | 	martin@NetBSD.org
 | Subject: Re: bin/52986: netpgpverify broken on sparc64
 | Date: Sun, 11 Feb 2018 16:18:03 +0000
 | 
 |  On Sun, Feb 11, 2018 at 03:40:01PM +0000, Martin Husemann wrote:
 |  >  (gdb) p *mem
 |  >  $24 = {size = 96, cc = 96, mem = 0xffffffffffffad08 "\210^\004\001\021\b",
 |  >    fp = 0x0, dealloc = 0 '\000', allowed = 0x1492b0 "\002\004\b\v"}
 |  >  
 |  >  here mem->mem is ok and we extract the proper hashalg a few bytes into that.
 |  
 |  But note that this memory is on the stack (or did I miscount the f's?) - I bet
 |  that is not planned to be so.

 it is mmapped so ...

 christos

From: Martin Husemann <martin@duskware.de>
To: Christos Zoulas <christos@zoulas.com>
Cc: gnats-bugs@NetBSD.org, agc@NetBSD.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/52986: netpgpverify broken on sparc64
Date: Mon, 12 Feb 2018 09:34:57 +0100

 On Sun, Feb 11, 2018 at 11:33:28AM -0500, Christos Zoulas wrote:
 > |  But note that this memory is on the stack (or did I miscount the f's?) - I bet
 > |  that is not planned to be so.
 > 
 > it is mmapped so ...

 No, in this case it is not (it is verifying a memory hash). I haven't found
 exactly where it is set, but I guess it living on the stack is a bug.

 However, there are two more bugs:

 Index: bcopy.c
 ===================================================================
 RCS file: /cvsroot/src/common/lib/libc/string/bcopy.c,v
 retrieving revision 1.11
 retrieving revision 1.12
 [..]
 @@ -89,7 +89,7 @@
         unsigned long u;

  #if !defined(_KERNEL)
 -       _DIAGASSERT((dst0 && src0) || length == 0);
 +       _DIAGASSERT(length == 0);
  #endif

 (inside the memmove implementation used by sparc64) always fires for
 the memmove's called by the BN lib here. This is obviously wrong,
 but: the firing assert does not terminate the program, but during handling
 of it somewhere the stack/memory corruption changing the encoded "hashalg"
 to zero happens.


 Martin

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52986 CVS commit: src/common/lib/libc/string
Date: Mon, 12 Feb 2018 11:14:15 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Mon Feb 12 11:14:15 UTC 2018

 Modified Files:
 	src/common/lib/libc/string: bcopy.c

 Log Message:
 Complete previous by complteley removing the _DIAGASSERT from memmove -
 the accidental left over from previous fired on all legitimate calls
 and caused PR bin/52986 and PR lib/52987.


 To generate a diff of this commit:
 cvs rdiff -u -r1.12 -r1.13 src/common/lib/libc/string/bcopy.c

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

State-Changed-From-To: open->feedback
State-Changed-By: martin@NetBSD.org
State-Changed-When: Mon, 12 Feb 2018 11:17:07 +0000
State-Changed-Why:
Works for me again after fixing the bogus assert in memmov() - not
sure if the other issues mentioned during discussion here are worth
closer examination.


From: christos@zoulas.com (Christos Zoulas)
To: Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@NetBSD.org, agc@NetBSD.org, gnats-admin@netbsd.org, 
	netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/52986: netpgpverify broken on sparc64
Date: Mon, 12 Feb 2018 07:33:31 -0500

 On Feb 12,  9:34am, martin@duskware.de (Martin Husemann) wrote:
 -- Subject: Re: bin/52986: netpgpverify broken on sparc64

 | No, in this case it is not (it is verifying a memory hash). I haven't found
 | exactly where it is set, but I guess it living on the stack is a bug.
 | 
 | However, there are two more bugs:
 | 
 | Index: bcopy.c
 | ===================================================================
 | RCS file: /cvsroot/src/common/lib/libc/string/bcopy.c,v
 | retrieving revision 1.11
 | retrieving revision 1.12
 | [..]
 | @@ -89,7 +89,7 @@
 |         unsigned long u;
 |  
 |  #if !defined(_KERNEL)
 | -       _DIAGASSERT((dst0 && src0) || length == 0);
 | +       _DIAGASSERT(length == 0);
 |  #endif
 | 
 | (inside the memmove implementation used by sparc64) always fires for
 | the memmove's called by the BN lib here. This is obviously wrong,
 | but: the firing assert does not terminate the program, but during handling
 | of it somewhere the stack/memory corruption changing the encoded "hashalg"
 | to zero happens.

 There is a bug somewhere else too that makes the unit tests fail on x86.
 I have not found out what it is yet, but changing the EVP_MD_CTX_new
 allocation in digest.c to add let's say 100 bytes of slop makes it go away.

 Does this fix your problem too?

 christos

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/52986: netpgpverify broken on sparc64
Date: Mon, 12 Feb 2018 13:37:16 +0100

 On Mon, Feb 12, 2018 at 12:35:02PM +0000, Christos Zoulas wrote:
 >  Does this fix your problem too?

 With working memmove() it does not fail for me anymore.

 Martin

State-Changed-From-To: feedback->closed
State-Changed-By: martin@NetBSD.org
State-Changed-When: Fri, 09 Mar 2018 14:45:36 +0000
State-Changed-Why:
Fixed, thanks!


>Unformatted:

Home	PR Database Search