Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752157AbaGBPLe (ORCPT ); Wed, 2 Jul 2014 11:11:34 -0400 Received: from mail-oa0-f48.google.com ([209.85.219.48]:58663 "EHLO mail-oa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753627AbaGBPLb (ORCPT ); Wed, 2 Jul 2014 11:11:31 -0400 MIME-Version: 1.0 In-Reply-To: <20140702134156.GL23978@titan.lakedaemon.net> References: <1404306020-24916-1-git-send-email-jason@lakedaemon.net> <20140702132635.GD7505@grmbl.mre> <20140702134156.GL23978@titan.lakedaemon.net> Date: Wed, 2 Jul 2014 08:11:30 -0700 X-Google-Sender-Auth: LlDxW0ugps9ku30oFXcDvloA2yY Message-ID: Subject: Re: [PATCH 1/2 v2] hwrng: Allow drivers to disable reading during probe From: Kees Cook To: Jason Cooper Cc: Amit Shah , LKML , "virtualization@lists.linux-foundation.org" , Herbert Xu , "# 3.4.x" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 2, 2014 at 6:41 AM, Jason Cooper wrote: > On Wed, Jul 02, 2014 at 06:56:35PM +0530, Amit Shah wrote: >> Hi Jason, >> >> On (Wed) 02 Jul 2014 [13:00:19], Jason Cooper wrote: >> > Commit d9e7972619334 "hwrng: add randomness to system from rng sources" >> > added a call to rng_get_data() from the hwrng_register() function. >> > However, some rng devices need initialization before data can be read >> > from them. >> > >> > Also, the virtio-rng device does not behave properly when this call is >> > made in its probe() routine - the virtio core sets the DRIVER_OK status >> > bit only on a successful probe, which means the host ignores all >> > communication from the guest, and the guest insmod or boot process just >> > sits there doing nothing. >> > >> > [ jac: Modify the API to allow drivers to disable reading at probe, new >> > patch, copied Amit's commit message. ] >> > >> > CC: Kees Cook >> > CC: Jason Cooper >> > CC: Herbert Xu >> > CC: # v3.15+ >> > Signed-off-by: Amit Shah >> > Signed-off-by: Jason Cooper >> > --- >> > drivers/char/hw_random/core.c | 8 +++++--- >> > include/linux/hw_random.h | 4 ++++ >> > 2 files changed, 9 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c >> > index 334601cc81cf..b7b6c48ca682 100644 >> > --- a/drivers/char/hw_random/core.c >> > +++ b/drivers/char/hw_random/core.c >> > @@ -347,9 +347,11 @@ int hwrng_register(struct hwrng *rng) >> > INIT_LIST_HEAD(&rng->list); >> > list_add_tail(&rng->list, &rng_list); >> > >> > - bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1); >> > - if (bytes_read > 0) >> > - add_device_randomness(bytes, bytes_read); >> > + if (!(rng->flags & HWRNG_NO_READ_AT_PROBE)) { >> > + bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1); >> > + if (bytes_read > 0) >> > + add_device_randomness(bytes, bytes_read); >> > + } >> >> But this has the inverse problem: if there are two hwrngs in the >> system, one will be initialized and probed. The other will not be >> initialized, but still be probed. > > That's a problem outside the scope of this patch. You're basically > saying the ->init() should be called unconditionally for each hwrng. If > that's what driver authors assumed, that's not what is happening if > there is more than one driver in the system. My intent with the patch was to get randomness added when a hwrng was available. Perhaps instead of skipping this call, it can be done either during probe (for devices that support it or lack an init function), or done after initialization? > I think you should be changing the code a few lines up to make sure > hwrng_init() is called once for each driver. Is that really how it should work? I'd be for the "always-init" approach since it gains us access to the entropy, but I have to play devil's advocate: would one expect "probe" to just probe, not initialize? That seems more like a function of enabling the device. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/