From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751124AbdAQW3d (ORCPT ); Tue, 17 Jan 2017 17:29:33 -0500 Received: from mga02.intel.com ([134.134.136.20]:56612 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750929AbdAQW3c (ORCPT ); Tue, 17 Jan 2017 17:29:32 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,246,1477983600"; d="scan'208";a="214528720" Subject: Re: random: /dev/random often returns short reads To: Denys Vlasenko , "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> From: "H. Peter Anvin" Message-ID: <71338f5a-83e3-4316-845d-8cdea735df0f@linux.intel.com> Date: Tue, 17 Jan 2017 14:29:30 -0800 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: <09f2ce2d-3c84-bb12-560c-3208691d2c55@redhat.com> Content-Type: multipart/mixed; boundary="------------0508325CE453385A4B1A0081" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is a multi-part message in MIME format. --------------0508325CE453385A4B1A0081 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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. You chould derate the value based on the top part of the threshold using a more conservative constant (using smaller fill steps) than the 3/4 used in the current derating algorithm, but first of all, you would only recover <= 1/4 of the credit in the first place, so it is questionable if it really buys you all that much. I really, really would hate to see something that introduces an active error to cope with a broken application somewhere. > On Mon, Jan 16, 2017 at 07:50:55PM +0100, Denys Vlasenko wrote: >> >> /dev/random can legitimately returns short reads >> when there is not enough entropy for the full request. > > Yes, but callers of /dev/random should be able to handle short reads. > So it's a bug in the application as well. It's not a bug in the application "as well", it is a bug in the application, *period*. There are a number of other conditions which could cause this exact effect. If there is a real need to hack around this, then I would instead suggest modifying random_read() to block rather than return if the user requests below a certain value, O_NONBLOCK is not set, and the whole request cannot be fulfilled. It probably needs to be a sysctl configurable, though, and most likely defaulting to 1, as it could just as easily break properly functioning applications. A *completely* untested patch attached... -hpa --------------0508325CE453385A4B1A0081 Content-Type: text/plain; charset=UTF-8; name="diff" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="diff" ZGlmZiAtLWdpdCBhL2RyaXZlcnMvY2hhci9yYW5kb20uYyBiL2RyaXZlcnMvY2hhci9yYW5k b20uYwppbmRleCAxZWYyNjQwLi42MThjYTliIDEwMDY0NAotLS0gYS9kcml2ZXJzL2NoYXIv cmFuZG9tLmMKKysrIGIvZHJpdmVycy9jaGFyL3JhbmRvbS5jCkBAIC0zMjAsNiArMzIwLDEz IEBAIHN0YXRpYyBpbnQgcmFuZG9tX3dyaXRlX3dha2V1cF9iaXRzID0gMjggKiBPVVRQVVRf UE9PTF9XT1JEUzsKIHN0YXRpYyBpbnQgcmFuZG9tX21pbl91cmFuZG9tX3NlZWQgPSA2MDsK IAogLyoKKyAqIElmIC9kZXYvcmFuZG9tIGNhbid0IGZ1bGZpbCBhIHJlcXVlc3QsIGJsb2Nr IHVubGVzcyB3ZSBjYW4gcmV0dXJuCisgKiB0aGlzIG1hbnkgYnl0ZXMuICBJZiBPX05PTkJM T0NLIGlzIHNldCwgd2UgYWx3YXlzIHJldHVybiwKKyAqIHVuY29uZGl0aW9uYWxseS4KKyAq Lworc3RhdGljIGludCByYW5kb21fbWluX3JldHVybiA9IDE7CisKKy8qCiAgKiBPcmlnaW5h bGx5LCB3ZSB1c2VkIGEgcHJpbWl0aXZlIHBvbHlub21pYWwgb2YgZGVncmVlIC5wb29sd29y ZHMKICAqIG92ZXIgR0YoMikuICBUaGUgdGFwcyBmb3IgdmFyaW91cyBzaXplcyBhcmUgZGVm aW5lZCBiZWxvdy4gIFRoZXkKICAqIHdlcmUgY2hvc2VuIHRvIGJlIGV2ZW5seSBzcGFjZWQg ZXhjZXB0IGZvciB0aGUgbGFzdCB0YXAsIHdoaWNoIGlzIDEKQEAgLTE3MDIsMjQgKzE3MDks MjYgQEAgc3RhdGljIHNzaXplX3QKIF9yYW5kb21fcmVhZChpbnQgbm9uYmxvY2ssIGNoYXIg X191c2VyICpidWYsIHNpemVfdCBuYnl0ZXMpCiB7CiAJc3NpemVfdCBuOworCXNpemVfdCBk b25lID0gMDsKIAogCWlmIChuYnl0ZXMgPT0gMCkKIAkJcmV0dXJuIDA7CiAKIAluYnl0ZXMg PSBtaW5fdChzaXplX3QsIG5ieXRlcywgU0VDX1hGRVJfU0laRSk7Ci0Jd2hpbGUgKDEpIHsK LQkJbiA9IGV4dHJhY3RfZW50cm9weV91c2VyKCZibG9ja2luZ19wb29sLCBidWYsIG5ieXRl cyk7CisJd2hpbGUgKGRvbmUgPCBuYnl0ZXMpIHsKKwkJbiA9IGV4dHJhY3RfZW50cm9weV91 c2VyKCZibG9ja2luZ19wb29sLCBidWYsIG5ieXRlcy1kb25lKTsKIAkJaWYgKG4gPCAwKQot CQkJcmV0dXJuIG47CisJCQlyZXR1cm4gZG9uZSA/IGRvbmUgOiBuOwogCQl0cmFjZV9yYW5k b21fcmVhZChuKjgsIChuYnl0ZXMtbikqOCwKIAkJCQkgIEVOVFJPUFlfQklUUygmYmxvY2tp bmdfcG9vbCksCiAJCQkJICBFTlRST1BZX0JJVFMoJmlucHV0X3Bvb2wpKTsKLQkJaWYgKG4g PiAwKQotCQkJcmV0dXJuIG47CisJCWRvbmUgKz0gbjsKKwkJaWYgKGRvbmUgPj0gbWluX3Qo c2l6ZV90LCBuYnl0ZXMsIHJhbmRvbV9taW5fcmVhZCkpCisJCQlicmVhazsKIAogCQkvKiBQ b29sIGlzIChuZWFyKSBlbXB0eS4gIE1heWJlIHdhaXQgYW5kIHJldHJ5LiAqLwogCQlpZiAo bm9uYmxvY2spCi0JCQlyZXR1cm4gLUVBR0FJTjsKKwkJCXJldHVybiBkb25lID8gZG9uZSA6 IC1FQUdBSU47CiAKIAkJd2FpdF9ldmVudF9pbnRlcnJ1cHRpYmxlKHJhbmRvbV9yZWFkX3dh aXQsCiAJCQlFTlRST1BZX0JJVFMoJmlucHV0X3Bvb2wpID49CkBAIC0xNzI3LDYgKzE3MzYs NyBAQCBfcmFuZG9tX3JlYWQoaW50IG5vbmJsb2NrLCBjaGFyIF9fdXNlciAqYnVmLCBzaXpl X3QgbmJ5dGVzKQogCQlpZiAoc2lnbmFsX3BlbmRpbmcoY3VycmVudCkpCiAJCQlyZXR1cm4g LUVSRVNUQVJUU1lTOwogCX0KKwlyZXR1cm4gZG9uZTsKIH0KIAogc3RhdGljIHNzaXplX3QK QEAgLTE5MDksNiArMTkxOSw4IEBAIFNZU0NBTExfREVGSU5FMyhnZXRyYW5kb20sIGNoYXIg X191c2VyICosIGJ1Ziwgc2l6ZV90LCBjb3VudCwKIAogI2luY2x1ZGUgPGxpbnV4L3N5c2N0 bC5oPgogCitzdGF0aWMgaW50IG1pbl9yYW5kb21fbWluX3JlYWQgPSAxOworc3RhdGljIGlu dCBtYXhfcmFuZG9tX21pbl9yZWFkID0gU0VDX1hGRVJfU0laRTsKIHN0YXRpYyBpbnQgbWlu X3JlYWRfdGhyZXNoID0gOCwgbWluX3dyaXRlX3RocmVzaDsKIHN0YXRpYyBpbnQgbWF4X3Jl YWRfdGhyZXNoID0gT1VUUFVUX1BPT0xfV09SRFMgKiAzMjsKIHN0YXRpYyBpbnQgbWF4X3dy aXRlX3RocmVzaCA9IElOUFVUX1BPT0xfV09SRFMgKiAzMjsKQEAgLTIwMjIsNiArMjAzNCwx NSBAQCBzdHJ1Y3QgY3RsX3RhYmxlIHJhbmRvbV90YWJsZVtdID0gewogCQkubW9kZQkJPSAw NDQ0LAogCQkucHJvY19oYW5kbGVyCT0gcHJvY19kb191dWlkLAogCX0sCisJeworCQkucHJv Y25hbWUJPSAicmFuZG9tX21pbl9yZXR1cm4iLAorCQkuZGF0YQkJPSAmcmFuZG9tX21pbl9y ZXR1cm4sCisJCS5tYXhsZW4JCT0gc2l6ZW9mKGludCksCisJCS5tb2RlCQk9IDA2NDQsCisJ CS5wcm9jX2hhbmRsZXIJPSBwcm9jX2RvaW50dmVjX21pbm1heCwKKwkJLmV4dHJhMQkJPSAm bWluX3JhbmRvbV9taW5fcmVhZCwKKwkJLmV4dHJhMgkJPSAmbWF4X3JhbmRvbV9taW5fcmVh ZCwKKwl9LAogI2lmZGVmIEFERF9JTlRFUlJVUFRfQkVOQ0gKIAl7CiAJCS5wcm9jbmFtZQk9 ICJhZGRfaW50ZXJydXB0X2F2Z19jeWNsZXMiLAo= --------------0508325CE453385A4B1A0081--