From owner-svn-src-user@freebsd.org Thu Oct 6 00:27:56 2016 Return-Path: Delivered-To: svn-src-user@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 9D1EBAF48DF for ; Thu, 6 Oct 2016 00:27:56 +0000 (UTC) (envelope-from pgollucci@p6m7g8.com) Received: from mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 24DC2B4 for ; Thu, 6 Oct 2016 00:27:56 +0000 (UTC) (envelope-from pgollucci@p6m7g8.com) Received: by mail-wm0-x22d.google.com with SMTP id b201so14081579wmb.0 for ; Wed, 05 Oct 2016 17:27:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=p6m7g8-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=wpObL4yyRuvDnJxo5UOHkiWeFlhHxY0gFSvNeGHmz+c=; b=NbfZ/dK0nsIohDAB3f5P681EawOVdAbouOfHMOBdtGiT1/YkzQJObXzqanJ3nEwsb1 dfz0cl8QbCz0urjsQ3pRtzlMSRqvfm2Z7p2p7yXmymLzqZoz15KmgkMQcvt9+bWbigb6 eBEEpvL44RUyKhwZy5itgZmGqySDT2cXKieFz9LPHBgQDDgEEEGH1asg8yGq6+RCBGoB LjLlIjfzXcKjl8IIAXHs5tWNuJv6fQXTp/TKmWKBz6LsJAMwei2MRXeKLtbPw8kz1B6N yT5nb5YqYNY9VNcbLYoDHHowFKIGUUUUgFC9GZxtrg0d4zqPnyhTk331zCf3MwWocstQ +0Dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=wpObL4yyRuvDnJxo5UOHkiWeFlhHxY0gFSvNeGHmz+c=; b=POrmBCk/kLbo2AC3X+EM5F/z0K5aY2f69IrmSKYb4QoJcCcL9UIkXvmgrKaN35tNBV EeNwKKC9oX1Ys8GWbEFmhPAXhH44woVl+OWCZ/OnWgtA7NaSIObYiSREH6Dql+qWkfAM rcrDudYc6ynr32Ti3y+QXPUNbvAAmMCe/9DYDFJvM79WFdoKu/Yocc++svaFVl/KlgXh x5Pfxy4IjBIT+63fULNd2m57wCHYNi1CR+/jH1W74TXtpOswUI2ytxXXlSyLPz/G8LOP hGe+M1Ix8mrfVr+gZU5mQhDHUo2cxG+/vJdT9MJWJ4vEr7R6JleqQIeZYoHPhv1hb9lw 5o7A== X-Gm-Message-State: AA6/9Rn1htekJsGTcvoxUbrDuYywzW/yRrZ/n/6BPjb5QfaTOB/Q4lHljRfhMcd6lJQuPQIIvGx69n1OVqi0Xg== X-Received: by 10.28.133.70 with SMTP id h67mr11487918wmd.61.1475713674688; Wed, 05 Oct 2016 17:27:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.80.183.197 with HTTP; Wed, 5 Oct 2016 17:27:14 -0700 (PDT) X-Originating-IP: [108.31.198.207] In-Reply-To: <20161006101333.N1163@besplex.bde.org> References: <201610051803.u95I3Hq1040052@repo.freebsd.org> <20161006101333.N1163@besplex.bde.org> From: "Philip M. Gollucci" Date: Wed, 5 Oct 2016 20:27:14 -0400 Message-ID: Subject: Re: svn commit: r306713 - in user/alc/PQ_LAUNDRY: lib/libc/stdlib lib/msun/ld80 lib/msun/src sys/cam sys/vm To: Bruce Evans Cc: Alan Cox , src-committers@freebsd.org, svn-src-user@freebsd.org Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Oct 2016 00:27:56 -0000 Duh, I missed that sysctl writes to it. My fault. You're of course correct. On Wed, Oct 5, 2016 at 8:13 PM, Bruce Evans wrote: > On Wed, 5 Oct 2016, Philip M. Gollucci wrote: > > I know you(Alan) didn't write this, its from the MFH, but how would len != >> expected. >> >> neither are initialized or passed in >> both times its set its expected = len = foo >> > > Er, both are initialized. len is passed as an input/output parameter to > sysctl(). sysctl() can return a short count. This is now detected by > recording the expected value in 'expected' and mishandled. Previously, > the error wasn't even detected. > > On Wed, Oct 5, 2016 at 2:03 PM, Alan Cox wrote: >> >> Modified: user/alc/PQ_LAUNDRY/lib/libc/stdlib/random.c >>> ============================================================ >>> ================== >>> --- user/alc/PQ_LAUNDRY/lib/libc/stdlib/random.c Wed Oct 5 >>> 17:32:06 2016 (r306712) >>> +++ user/alc/PQ_LAUNDRY/lib/libc/stdlib/random.c Wed Oct 5 >>> 18:03:17 2016 (r306713) >>> @@ -270,16 +270,17 @@ void >>> srandomdev(void) >>> { >>> int mib[2]; >>> - size_t len; >>> + size_t expected, len; >>> >>> if (rand_type == TYPE_0) >>> - len = sizeof(state[0]); >>> + expected = len = sizeof(state[0]); >>> else >>> - len = rand_deg * sizeof(state[0]); >>> + expected = len = rand_deg * sizeof(state[0]); >>> >>> mib[0] = CTL_KERN; >>> mib[1] = KERN_ARND; >>> - sysctl(mib, 2, state, &len, NULL, 0); >>> + if (sysctl(mib, 2, state, &len, NULL, 0) == -1 || len != >>> expected) >>> + abort(); >>> >>> if (rand_type != TYPE_0) { >>> fptr = &state[rand_sep]; >>> >> > I don't like this set of changes. > > Originally, srandomdev() read from /dev/random. read() can also return a > short count. This was detected and mishandled, but not as badly as now. > The non-error of a short count was treated as an error, and both were > handled by falling back to the weak method of using gettimeofday(). > > This was broken by switching to the sysctl and not even checking for > errors from the sysctl. The sysctl can and does fail, mainly when a > new userland is used with an old kernel. > > This commit restores some error checking. > > The implementation uses the style bugs of using the numbered sysctl > KERN_ARND. This sysctl shouldn't exist by number, and isn't documented > by number. It is too new to need a number or benefit from the careful > documentation of old numbered sysctls. It is only documented by name > (kern.arandom), only in a wrong place (random(4)), and a naive reader > would expect from reading this man page that /dev/random is still the > primary interface. Its behaviour of returning a short count might be > expected from the device's behaviour, but is not really documented for > the sysctl. It is only documented that the sysctl will not return > random bytes unless the random device is seeded. The return value for > this case is undocumented (is it 0 or -1? Source code seems to say > that it is 0). Anyway, errors seems to be possible, so abort() is not > acceptable handling. > > This was handled much better in arc4random() and is still handled better > there despite recent regressions: > - the sysctl is wrapped by a function that retries for short counts. > However, the sysctl usage has even larger style bugs -- the sysctl is > by number, and sysctl() is named __sysctl() > - error handling was not missing. Perhaps the retry loop can spin forever > with a short count of 0, but if the sysctl fails then there was a > fallback > first to reading /dev/random and then to the weak gettimeofday() method. > > Now there is no fallback, and the difference is just the retry loop. > > The first step in the removed fallback in arc4random() was not weaker > like the log message says. It is to the primary method according to > the man page. According to the man page, reading /dev/random blocks > until the RNG is seeded for the first time. Blocking for the sysctl > is undocumented. I think the sysctl returns a short count of 0, and > we treat this as an error and abort(), so applications running early > in the boot crash instead of having the traditional behaviour of hanging. > > It is unclear if short counts of nonzero can actually occur. I think > they could easily occur in older implementations where /dev/random > blocked for more cases than before the initial seeding. I think they > should still be possible. Even if there is no need to block, it is > useful to be able to kill a read() with a preposterously large count. > If interrupts are allowed at all, then after one the return value must > be either -1 or a short count. That is another reason why abort() in > a library is bad error handling. It turns most signals into SIGABRT. > > Bruce > -- --------------------------------------------------------------------------------- 4096R/D21D2752 ECDF B597 B54B 7F92 753E E0EA F699 A450 D21D 2752 Philip M. Gollucci (pgollucci@p6m7g8.com) c: 703.336.9354 Member, Apache Software Foundation Committer, FreeBSD Foundation Consultant, P6M7G8 Inc. Director Cloud Technology, Capital One What doesn't kill us can only make us stronger; Except it almost kills you.