NetBSD Problem Report #46751

From www@NetBSD.org  Sat Jul 28 21:30:37 2012
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id 95C2D63B8E6
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 28 Jul 2012 21:30:37 +0000 (UTC)
Message-Id: <20120728213037.0E8DE63B85F@www.NetBSD.org>
Date: Sat, 28 Jul 2012 21:30:37 +0000 (UTC)
From: matt.a.martin@gmail.com
Reply-To: matt.a.martin@gmail.com
To: gnats-bugs@NetBSD.org
Subject: arc4random_addrandom(3) always reinitializes
X-Send-Pr-Version: www-1.0

>Number:         46751
>Category:       lib
>Synopsis:       arc4random_addrandom(3) always reinitializes
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jul 28 21:35:00 +0000 2012
>Closed-Date:    Thu May 26 03:50:11 +0000 2022
>Last-Modified:  Thu May 26 03:50:11 +0000 2022
>Originator:     matt martin
>Release:        
>Organization:
>Environment:
>Description:
A negation was dropped (looks like in 1.11 -> 1.12)

static inline void
_arc4random_addrandom_unlocked(u_char *dat, int datlen)
{
	if (__predict_false(rs.initialized)) { // <-- here
		arc4_init(&rs);
	}
	arc4_addrandom(&rs, dat, datlen);
}

Since arc4_init() calls arc4_stir() and calling anything else will wind up calling arc4_init() this doesn't seem to break anything. Looks like it'll just throw away whatever changes arc4random_addrandom() made if nothing had (ever...) been called before it.
>How-To-Repeat:
Looking at http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/lib/libc/gen/arc4random.c?rev=1.13&content-type=text/plain
>Fix:
Replace the negation.

static inline void
_arc4random_addrandom_unlocked(u_char *dat, int datlen)
{
	if (__predict_false(!rs.initialized)) {
		arc4_init(&rs);
	}
	arc4_addrandom(&rs, dat, datlen);
}

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->closed
State-Changed-By: dsl@NetBSD.org
State-Changed-When: Sat, 18 Aug 2012 15:57:28 +0000
State-Changed-Why:
Fixed by rev 1.15 of lib/libc/gen/arc4random.c


From: Matthew Mondor <mm_lists@pulsar-zone.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/46751 (arc4random_addrandom(3) always reinitializes)
Date: Sun, 19 Aug 2012 21:40:34 -0400

 On Sat, 18 Aug 2012 15:57:29 +0000 (UTC)
 dsl@NetBSD.org wrote:

 > Synopsis: arc4random_addrandom(3) always reinitializes
 > 
 > State-Changed-From-To: open->closed
 > State-Changed-By: dsl@NetBSD.org
 > State-Changed-When: Sat, 18 Aug 2012 15:57:28 +0000
 > State-Changed-Why:
 > Fixed by rev 1.15 of lib/libc/gen/arc4random.c

 It appears that netbsd-6 still has 1.10, and that 1.12 fixes a security
 issue (which according to the commit comment also affects 1.10).

 Should some recent arc4random.c fixes be pulled up to netbsd-6?

 Thanks,
 -- 
 Matt

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/46751 (arc4random_addrandom(3) always reinitializes)
Date: Mon, 20 Aug 2012 08:28:30 +0100

 On Mon, Aug 20, 2012 at 01:45:02AM +0000, Matthew Mondor wrote:
 > Subject: Re: lib/46751 (arc4random_addrandom(3) always reinitializes)
 >  
 >  It appears that netbsd-6 still has 1.10, and that 1.12 fixes a security
 >  issue (which according to the commit comment also affects 1.10).
 >  
 >  Should some recent arc4random.c fixes be pulled up to netbsd-6?

 Looking at a diff of 1.10 to 1.16 (they are more similar than any
 of the intervening versions!) shows that there are 2 main changes:
 1) the LOCK/UNLOCK pairs for threaded programs
 2) the initialisation call in arc4random_buf()

 The latter might be deemed important!
 arc4random_buf() didn't exist before rev 1.10.

 Getting a bit late for pullups :-)

 	David

 -- 
 David Laight: david@l8s.co.uk

State-Changed-From-To: closed->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Wed, 22 Aug 2012 06:51:50 +0000
State-Changed-Why:
this ought to be fixed in -6.


From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/46751 (arc4random_addrandom(3) always reinitializes)
Date: Wed, 22 Aug 2012 06:50:52 +0000

 On Mon, Aug 20, 2012 at 01:45:02AM +0000, Matthew Mondor wrote:
  >  > State-Changed-Why:
  >  > Fixed by rev 1.15 of lib/libc/gen/arc4random.c
  >  
  >  It appears that netbsd-6 still has 1.10, and that 1.12 fixes a security
  >  issue (which according to the commit comment also affects 1.10).

 You have misread, I think - 1.11 and 1.12 were pulled up in March, and
 the version in netbsd-6 is the same as 1.12.

  >  Should some recent arc4random.c fixes be pulled up to netbsd-6?

 Possibly. The problem in this PR does exist in 1.12; however, it might
 be best to fix it in -6 just by inserting the missing ! in the
 affected conditional.

 -- 
 David A. Holland
 dholland@netbsd.org

From: Matthew Mondor <mm_lists@pulsar-zone.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/46751 (arc4random_addrandom(3) always reinitializes)
Date: Wed, 22 Aug 2012 03:15:17 -0400

 On Wed, 22 Aug 2012 06:55:01 +0000 (UTC)
 David Holland <dholland-bugs@netbsd.org> wrote:

 >   >  It appears that netbsd-6 still has 1.10, and that 1.12 fixes a security
 >   >  issue (which according to the commit comment also affects 1.10).
 >  
 >  You have misread, I think - 1.11 and 1.12 were pulled up in March, and
 >  the version in netbsd-6 is the same as 1.12.

 You're right, I was checking on the MAIN branch instead of the netbsd-6
 one, where I see 1.10.6.1 including the pullup.

 Thanks,
 -- 
 Matt

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Thu, 26 May 2022 03:50:11 +0000
State-Changed-Why:
pullups to -6 are long irrelevant


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.