From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C1886C433F5 for ; Fri, 14 Jan 2022 15:06:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Q3jDp6FG8uX6H9T8Viv/7z/bVP7LqnpD9xpk+NMSIVE=; b=yxUZIk29++og0a rGWuaa46MdZ/gvuOLRerP1W54r1PPrHVYQrp7xIzaZ9yvwdJBJZ2RYk1vdHY3tX9XOI4lrR33Sytj ouTEUBUyREoaQq6a9QYi+H4BkaPliTJbPg8UQHVpHu8apoxrDo7kKcz/qSCgBzF0b+aSA3Xy0sbsb UrrXQ/J+ClGy3Q/IvXjwAbh+yVEj6s2yrU+Xkw7oSdjCchjjWOWtYtopDcudgkTXi087FlTneAxB2 aSaK3OjCgvIxCTeBl4RpZZ/xt9AjmpgT63NL081siTAfA0Dldv4p9TwH06HQDdgE9d0Wagi/+u/cK Yd+JxVGONtIQ/mmzqf8A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n8O8G-009USt-6o; Fri, 14 Jan 2022 15:04:48 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n8O8B-009UQC-Eb for linux-arm-kernel@lists.infradead.org; Fri, 14 Jan 2022 15:04:45 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8349B6D; Fri, 14 Jan 2022 07:04:40 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 76D433F774; Fri, 14 Jan 2022 07:04:39 -0800 (PST) Date: Fri, 14 Jan 2022 15:04:37 +0000 From: Andre Przywara To: "Jason A. Donenfeld" Cc: Ard Biesheuvel , linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, will@kernel.org, mark.rutland@arm.com, Mark Brown Subject: Re: [PATCH] arm64: random: implement arch_get_random_int/_long based on RNDR Message-ID: <20220114150437.1542ccbe@donnerap.cambridge.arm.com> In-Reply-To: References: <20220113131239.1610455-1-ardb@kernel.org> Organization: ARM X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220114_070443_637855_D11FB1C4 X-CRM114-Status: GOOD ( 33.16 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 13 Jan 2022 14:41:16 +0100 "Jason A. Donenfeld" wrote: Hi Jason, > Wow, didn't expect for this to come so fast. Excellent. > > On Thu, Jan 13, 2022 at 02:12:39PM +0100, Ard Biesheuvel wrote: > > - map arch_get_random_int/_long() onto RNDR, which returns the output of > > a DRBG that is reseeded at an implemented defined rate; > > implemented -> implementation? > > > static inline bool __must_check arch_get_random_long(unsigned long *v) > > { > > + /* > > + * Only support the generic interface after we have detected > > + * the system wide capability, avoiding complexity with the > > + * cpufeature code and with potential scheduling between CPUs > > + * with and without the feature. > > + */ > > + if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v)) > > + return true; > > return false; > > } > > Can't this just become: > > return cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v); It could, but this pattern is used in the other functions to simplify having extensions later. > > > > static inline bool __must_check arch_get_random_int(unsigned int *v) > > { > > + if (cpus_have_const_cap(ARM64_HAS_RNG)) { > > + unsigned long val; > > + > > + if (__arm64_rndr(&val)) { > > + *v = val; > > + return true; > > + } > > + } > > return false; > > } > > Why not implement arch_get_random_int with the same type of flow as > arch_get_random_long? > > static inline bool __must_check arch_get_random_int(unsigned int *v) > { > unsigned long val; > if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(&val))) { > *v = val; > return true; > } > return false; > } > > Or, even better, just define arch_get_random_int in terms of > arch_get_random_long: arch_get_random_long() might drain more entropy than needed, at least this is a problem for the TRNG version. For the RNDR* instructions this doesn't really matter, since they are always using 64-bit system registers, but for the SMC interface there is a difference between asking for 32 or 64 bits. So this is admittedly not a problem in *this particular case*, but I'd consider this an implementation detail of the sources used in the current code, and for the sake of expandability it seems more robust to use this approach. Cheers, Andre > > static inline bool __must_check arch_get_random_int(unsigned int *v) > { > unsigned long val; > if (arch_get_random_long(&val)) { > *v = val; > return true; > } > return false; > } > > > > @@ -71,12 +105,11 @@ static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > > } > > > > /* > > - * Only support the generic interface after we have detected > > - * the system wide capability, avoiding complexity with the > > - * cpufeature code and with potential scheduling between CPUs > > - * with and without the feature. > > + * RNDRRS is not backed by an entropy source but by a DRBG that is > > + * reseeded after each invocation. This is not a 100% fit but good > > + * enough to implement this API if no other entropy source exists. > > The docs are actually a bit more optimistic than that: > > https://developer.arm.com/documentation/ddi0595/2021-03/AArch64-Registers/RNDRRS--Reseeded-Random-Number > > ~ Reseeded Random Number. Returns a 64-bit random number which is reseeded > ~ from the True Random Number source immediately before the read of the > ~ random number. > > If I'm reading that correctly, it looks like the reseeding happens > *before* the read, and it looks like it comes from a TRNG. In > other words, it sounds to me like it's just doing something like > HASH(READ_TRNG()). That would be pretty darn good. > > > */ > > - if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndr(v)) > > + if (cpus_have_const_cap(ARM64_HAS_RNG) && __arm64_rndrrs(v)) > > return true; > > > > return false; > > @@ -96,7 +129,7 @@ static inline bool __must_check arch_get_random_seed_int(unsigned int *v) > > } > > > > if (cpus_have_const_cap(ARM64_HAS_RNG)) { > > - if (__arm64_rndr(&val)) { > > + if (__arm64_rndrrs(&val)) { > > *v = val; > > return true; > > } > > I suppose the same control flow simplification stuff mentioned above > could be done here too, if you feel like what I mentioned earlier is > worthwhile. > > From a randomness perspective: > > Acked-by: Jason A. Donenfeld > > Thanks, > Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel