From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752854AbdARPwg (ORCPT ); Wed, 18 Jan 2017 10:52:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39866 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752773AbdARPwd (ORCPT ); Wed, 18 Jan 2017 10:52:33 -0500 Subject: Re: random: /dev/random often returns short reads To: "H. Peter Anvin" , "Theodore Ts'o" , Denys Vlasenko , Linux Kernel Mailing List References: <20170117043640.4ykofgcwfysvgyue@thunk.org> <20170117171539.dadciiz2kfjtqrfk@thunk.org> <09f2ce2d-3c84-bb12-560c-3208691d2c55@redhat.com> <71338f5a-83e3-4316-845d-8cdea735df0f@linux.intel.com> From: Denys Vlasenko Message-ID: <2bc270ee-9f42-b4b3-a347-3a7a758d5d1f@redhat.com> Date: Wed, 18 Jan 2017 16:44:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <71338f5a-83e3-4316-845d-8cdea735df0f@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 18 Jan 2017 15:44:56 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/17/2017 11:29 PM, H. Peter Anvin wrote: > On 01/17/17 09:34, Denys Vlasenko wrote: >> >> >> On 01/17/2017 06:15 PM, Theodore Ts'o wrote: >>> On Tue, Jan 17, 2017 at 09:21:31AM +0100, Denys Vlasenko wrote: >>>>> If someone wants to send me a patch, I'll happily take a look at it, >>>> >>>> Will something along these lines be accepted? >>> >>> The problem is that this won't work. In the cases that we're talking >>> about, the entropy counter in the secondary pool is not zero, but >>> close to zero, we'll still have short reads. And that's going to >>> happen a fair amount of the time. >>> >>> Perhaps the best *hacky* solution would be to say, ok if the entropy >>> count is less than some threshold, don't use the correct entropy >>> calculation, but rather assume that all of the new bits won't land on >>> top of existing entropy bits. >> >> IOW, something like this: >> >> --- a/drivers/char/random.c >> +++ b/drivers/char/random.c >> @@ -653,6 +653,9 @@ static void credit_entropy_bits(struct >> entropy_store *r, int nbits) >> if (nfrac < 0) { >> /* Debit */ >> entropy_count += nfrac; >> + } else if (entropy_count < ((8 * 8) << ENTROPY_SHIFT)) { >> + /* Credit, and the pool is almost empty */ >> + entropy_count += nfrac; >> } else { >> /* >> * Credit: we have to account for the possibility of >> * overwriting already present entropy. Even in the >> >> Want the patch? If yes, what name of the constant you prefer? How about >> > > This seems very wrong. The whole point is that we keep it conservative > -- always less than or equal to the correct number. In this case, what code does is it returns fewer bytes, even though *it has enough random bytes to return the full request*. This happens because the patch which added more conservative accounting, while containing technically correct accounting per se, it forgot to take in the account another part of the code which was relying on the previous, simplistic logic "if we add N random bytes to a pool, now it has +N random bytes". In my opinion, it should have changed that part of code simultaneously with introducing more conservative accounting.