From patchwork Tue Dec 13 14:10:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corentin Labbe X-Patchwork-Id: 9472357 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AD4F260760 for ; Tue, 13 Dec 2016 14:13:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9CC7028543 for ; Tue, 13 Dec 2016 14:13:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9147628560; Tue, 13 Dec 2016 14:13:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,FREEMAIL_FROM,RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B70DD28543 for ; Tue, 13 Dec 2016 14:13:14 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1cGnnr-0004CG-18; Tue, 13 Dec 2016 14:11:35 +0000 Received: from mail-wj0-x243.google.com ([2a00:1450:400c:c01::243]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1cGnng-00042x-Pi for linux-arm-kernel@lists.infradead.org; Tue, 13 Dec 2016 14:11:27 +0000 Received: by mail-wj0-x243.google.com with SMTP id j10so16757092wjb.3 for ; Tue, 13 Dec 2016 06:11:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Bpvh1vexsbJ7sgwW5udU2QCT6c+93xIh5osHflN+6+o=; b=Y2ec4sCfxG388btOGeN0M/Z/fCLjG5fsjmvomxrirqkSQoVXJNRA2jSVoTZKwf1wKq DgV75My8r/kQ2AGbd94Tt9QzOiGRk1AARfX+ee1XWmPD9KZpgBv4FI69CRnoYoh2p3AY L4nsDH2OA3pAIEdN2hvGd7KM6BKkUwHkM4yWLroasWGpdwdN0HSTjdrgSS75KPgupQb/ u6EzBlm/jBJ7aSe399y/No4I+V7r5Hvev59c62sWI91xNqzb2b7eap5433Xx/Vi6k3X2 KP6EqSxXnMy0hM9Cx6uHtadYDkEFAsMbTE9unf2xx4V1N5Xg1H+Ogi17EirGxFK/SkUQ sB1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Bpvh1vexsbJ7sgwW5udU2QCT6c+93xIh5osHflN+6+o=; b=S029aiYt70c6/lt50wEkpeRLbEc+bDhDWwxBw+JFe3ACoQTUG2Q1huMgyZowWHDRno diS3OVUCpjdgdth0Da9VZ8JaIAmMNXhUnccyv2o7iFCgZQFGSWGexFSa1YB5XZOwY4hV DrgXZUdI60iDVRv2veNY6evKAHgQWjfb0d8xoCiZ9gcw1fvql3BCNBPoeSfR8D/kIbcl W+YtHv8Kjdh6SzIJiVTjfBhM/FVyV8nFoKW5YQwsQMWZj7Pr19Rkmr5AN1hAsZGRjYLN LtNI7VneA4+kqpd0KnUo9Aw3wN25dfe5HOBX2uDfZZbqyyNsYkLXbNvv9ljATWASclNE pnDQ== X-Gm-Message-State: AKaTC02YDQ0DL440bcC0cYzC9BSZjD4z+aULwWAoOvUxir8nV7ZIZuJmzIpMRilIlZ+xTg== X-Received: by 10.194.127.40 with SMTP id nd8mr96889133wjb.43.1481638262679; Tue, 13 Dec 2016 06:11:02 -0800 (PST) Received: from Red (LFbn-1-7035-57.w90-116.abo.wanadoo.fr. [90.116.208.57]) by smtp.googlemail.com with ESMTPSA id i10sm62447088wjd.15.2016.12.13.06.11.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Dec 2016 06:11:01 -0800 (PST) Date: Tue, 13 Dec 2016 15:10:59 +0100 From: Corentin Labbe To: Herbert Xu Subject: Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG Message-ID: <20161213141059.GB10647@Red> References: <1480934922-20732-1-git-send-email-clabbe.montjoie@gmail.com> <20161205123705.GA10732@gondor.apana.org.au> <20161205125738.GA13525@Red> <20161207120900.GC20680@gondor.apana.org.au> <20161207125127.GB28218@Red> <20161208090618.GB22932@gondor.apana.org.au> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161208090618.GB22932@gondor.apana.org.au> User-Agent: Mutt/1.5.24 (2015-08-30) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20161213_061125_268211_5F049E77 X-CRM114-Status: GOOD ( 28.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-kernel@vger.kernel.org, wens@csie.org, linux-crypto@vger.kernel.org, maxime.ripard@free-electrons.com, davem@davemloft.net, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Dec 08, 2016 at 05:06:18PM +0800, Herbert Xu wrote: > On Wed, Dec 07, 2016 at 01:51:27PM +0100, Corentin Labbe wrote: > > > > So I must expose it as a crypto_rng ? > > If it is to be exposed at all then algif_rng would be the best > place. > > > Could you explain why PRNG must not be used as hw_random ? > > The hwrng interface was always meant to be an interface for real > hardware random number generators. People rely on that so we > should not provide bogus entropy sources through this interface. > > Cheers, I have found two solutions: The simplier is to add an attribute isprng to hwrng like that: Regards Corentin Labbe --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -150,7 +150,8 @@ static int hwrng_init(struct hwrng *rng) reinit_completion(&rng->cleanup_done); skip_init: - add_early_randomness(rng); + if (!rng->isprng) + add_early_randomness(rng); current_quality = rng->quality ? : default_quality; if (current_quality > 1024) @@ -158,7 +159,7 @@ static int hwrng_init(struct hwrng *rng) if (current_quality == 0 && hwrng_fill) kthread_stop(hwrng_fill); - if (current_quality > 0 && !hwrng_fill) + if (current_quality > 0 && !hwrng_fill && !rng->isprng) start_khwrngd(); return 0; @@ -439,7 +440,7 @@ int hwrng_register(struct hwrng *rng) } list_add_tail(&rng->list, &rng_list); - if (old_rng && !rng->init) { + if (old_rng && !rng->init && !rng->isprng) { /* * Use a new device's input to add some randomness to * the system. If this rng device isn't going to be diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h index 34a0dc1..5a5b8dc 100644 --- a/include/linux/hw_random.h +++ b/include/linux/hw_random.h @@ -50,6 +50,7 @@ struct hwrng { struct list_head list; struct kref ref; struct completion cleanup_done; + bool isprng; }; With that, we still could register prng, but they dont provide any entropy. An optional Kconfig/"module parameter" could still be added for people still wanting this old behavour. The other solution is to "duplicate" /dev/hwrng to /dev/hwprng like that: --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -24,8 +24,11 @@ #include #define RNG_MODULE_NAME "hw_random" +#define PRNG_MODULE_NAME "hw_prng" +#define HWPRNG_MINOR 185 /* not official */ static struct hwrng *current_rng; +static struct hwrng *current_prng; static struct task_struct *hwrng_fill; static LIST_HEAD(rng_list); /* Protects rng_list and current_rng */ @@ -44,7 +47,7 @@ module_param(default_quality, ushort, 0644); MODULE_PARM_DESC(default_quality, "default entropy content of hwrng per mill"); -static void drop_current_rng(void); +static void drop_current_rng(bool prng); static int hwrng_init(struct hwrng *rng); static void start_khwrngd(void); @@ -56,6 +59,14 @@ static size_t rng_buffer_size(void) return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES; } +static bool is_current_xrng(struct hwrng *rng) +{ + if (rng->isprng) + return (rng == current_prng); + else + return (rng == current_rng); +} + static void add_early_randomness(struct hwrng *rng) { int bytes_read; @@ -88,32 +99,46 @@ static int set_current_rng(struct hwrng *rng) if (err) return err; - drop_current_rng(); - current_rng = rng; + drop_current_rng(rng->isprng); + if (rng->isprng) + current_prng = rng; + else + current_rng = rng; return 0; } -static void drop_current_rng(void) +static void drop_current_rng(bool prng) { BUG_ON(!mutex_is_locked(&rng_mutex)); - if (!current_rng) - return; - - /* decrease last reference for triggering the cleanup */ - kref_put(¤t_rng->ref, cleanup_rng); - current_rng = NULL; + if (prng) { + if (!current_prng) + return; + /* decrease last reference for triggering the cleanup */ + kref_put(¤t_prng->ref, cleanup_rng); + current_prng = NULL; + } else { + if (!current_rng) + return; + /* decrease last reference for triggering the cleanup */ + kref_put(¤t_rng->ref, cleanup_rng); + current_rng = NULL; + } } /* Returns ERR_PTR(), NULL or refcounted hwrng */ -static struct hwrng *get_current_rng(void) +static struct hwrng *get_current_rng(bool prng) { struct hwrng *rng; if (mutex_lock_interruptible(&rng_mutex)) return ERR_PTR(-ERESTARTSYS); - rng = current_rng; + if (prng) + rng = current_prng; + else + rng = current_rng; + if (rng) kref_get(&rng->ref); @@ -193,8 +218,8 @@ static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size, return 0; } -static ssize_t rng_dev_read(struct file *filp, char __user *buf, - size_t size, loff_t *offp) +static ssize_t genrng_dev_read(struct file *filp, char __user *buf, + size_t size, loff_t *offp, bool prng) { ssize_t ret = 0; int err = 0; @@ -202,7 +227,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf, struct hwrng *rng; while (size) { - rng = get_current_rng(); + rng = get_current_rng(prng); if (IS_ERR(rng)) { err = PTR_ERR(rng); goto out; @@ -270,6 +295,18 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf, goto out; } +static ssize_t rng_dev_read(struct file *filp, char __user *buf, + size_t size, loff_t *offp) +{ + return genrng_dev_read(filp, buf, size, offp, false); +} + +static ssize_t prng_dev_read(struct file *filp, char __user *buf, + size_t size, loff_t *offp) +{ + return genrng_dev_read(filp, buf, size, offp, true); +} + static const struct file_operations rng_chrdev_ops = { .owner = THIS_MODULE, .open = rng_dev_open, @@ -278,6 +315,7 @@ static const struct file_operations rng_chrdev_ops = { }; static const struct attribute_group *rng_dev_groups[]; +static const struct attribute_group *prng_dev_groups[]; static struct miscdevice rng_miscdev = { .minor = HWRNG_MINOR, @@ -287,9 +325,24 @@ static struct miscdevice rng_miscdev = { .groups = rng_dev_groups, }; -static ssize_t hwrng_attr_current_store(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t len) +static const struct file_operations prng_chrdev_ops = { + .owner = THIS_MODULE, + .open = rng_dev_open, + .read = prng_dev_read, + .llseek = noop_llseek, +}; + +static struct miscdevice prng_miscdev = { + .minor = HWPRNG_MINOR, + .name = PRNG_MODULE_NAME, + .nodename = "hwprng", + .fops = &prng_chrdev_ops, + .groups = prng_dev_groups, +}; + +static ssize_t genrng_attr_current_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len, bool prng) { int err; struct hwrng *rng; @@ -301,8 +354,13 @@ static ssize_t hwrng_attr_current_store(struct device *dev, list_for_each_entry(rng, &rng_list, list) { if (sysfs_streq(rng->name, buf)) { err = 0; - if (rng != current_rng) - err = set_current_rng(rng); + if (prng) { + if (rng != current_prng) + err = set_current_rng(rng); + } else { + if (rng != current_rng) + err = set_current_rng(rng); + } break; } } @@ -311,14 +369,28 @@ static ssize_t hwrng_attr_current_store(struct device *dev, return err ? : len; } -static ssize_t hwrng_attr_current_show(struct device *dev, - struct device_attribute *attr, - char *buf) +static ssize_t hwrng_attr_current_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + return genrng_attr_current_store(dev, attr, buf, len, false); +} + +static ssize_t hwprng_attr_current_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + return genrng_attr_current_store(dev, attr, buf, len, true); +} + +static ssize_t genrng_attr_current_show(struct device *dev, + struct device_attribute *attr, + char *buf, bool prng) { ssize_t ret; struct hwrng *rng; - rng = get_current_rng(); + rng = get_current_rng(prng); if (IS_ERR(rng)) return PTR_ERR(rng); @@ -328,9 +400,24 @@ static ssize_t hwrng_attr_current_show(struct device *dev, return ret; } -static ssize_t hwrng_attr_available_show(struct device *dev, +static ssize_t hwrng_attr_current_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return genrng_attr_current_show(dev, attr, buf, false); +} + +static ssize_t hwprng_attr_current_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return genrng_attr_current_show(dev, attr, buf, true); +} + + +static ssize_t hwgenrng_attr_available_show(struct device *dev, struct device_attribute *attr, - char *buf) + char *buf, bool prng) { int err; struct hwrng *rng; @@ -340,8 +427,10 @@ static ssize_t hwrng_attr_available_show(struct device *dev, return -ERESTARTSYS; buf[0] = '\0'; list_for_each_entry(rng, &rng_list, list) { - strlcat(buf, rng->name, PAGE_SIZE); - strlcat(buf, " ", PAGE_SIZE); + if (rng->isprng == prng) { + strlcat(buf, rng->name, PAGE_SIZE); + strlcat(buf, " ", PAGE_SIZE); + } } strlcat(buf, "\n", PAGE_SIZE); mutex_unlock(&rng_mutex); @@ -349,6 +438,20 @@ static ssize_t hwrng_attr_available_show(struct device *dev, return strlen(buf); } +static ssize_t hwrng_attr_available_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return hwgenrng_attr_available_show(dev, attr, buf, false); +} + +static ssize_t hwprng_attr_available_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + return hwgenrng_attr_available_show(dev, attr, buf, true); +} + static DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR, hwrng_attr_current_show, hwrng_attr_current_store); @@ -356,6 +459,13 @@ static DEVICE_ATTR(rng_available, S_IRUGO, hwrng_attr_available_show, NULL); +static DEVICE_ATTR(prng_current, S_IRUGO | S_IWUSR, + hwprng_attr_current_show, + hwprng_attr_current_store); +static DEVICE_ATTR(prng_available, S_IRUGO, + hwprng_attr_available_show, + NULL); + static struct attribute *rng_dev_attrs[] = { &dev_attr_rng_current.attr, &dev_attr_rng_available.attr, @@ -364,14 +474,35 @@ static struct attribute *rng_dev_attrs[] = { ATTRIBUTE_GROUPS(rng_dev); +static struct attribute *prng_dev_attrs[] = { + &dev_attr_prng_current.attr, + &dev_attr_prng_available.attr, + NULL +}; + +ATTRIBUTE_GROUPS(prng_dev); + static void __exit unregister_miscdev(void) { misc_deregister(&rng_miscdev); + misc_deregister(&prng_miscdev); } static int __init register_miscdev(void) { - return misc_register(&rng_miscdev); + int err; + + err = misc_register(&rng_miscdev); + if (err) + return err; + err = misc_register(&prng_miscdev); + if (err) + goto reg_error; + else + return 0; +reg_error: + misc_deregister(&rng_miscdev); + return err; } static int hwrng_fillfn(void *unused) @@ -381,7 +512,7 @@ static int hwrng_fillfn(void *unused) while (!kthread_should_stop()) { struct hwrng *rng; - rng = get_current_rng(); + rng = get_current_rng(false); if (IS_ERR(rng) || !rng) break; mutex_lock(&reading_mutex); @@ -462,8 +593,8 @@ void hwrng_unregister(struct hwrng *rng) mutex_lock(&rng_mutex); list_del(&rng->list); - if (current_rng == rng) { - drop_current_rng(); + if (is_current_xrng(rng)) { + drop_current_rng(rng->isprng); if (!list_empty(&rng_list)) { struct hwrng *tail; @@ -553,7 +684,8 @@ static int __init hwrng_modinit(void) static void __exit hwrng_modexit(void) { mutex_lock(&rng_mutex); - BUG_ON(current_rng); + WARN_ON(current_rng); + WARN_ON(current_prng); kfree(rng_buffer); kfree(rng_fillbuf); mutex_unlock(&rng_mutex); diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h index 34a0dc1..5a5b8dc 100644 --- a/include/linux/hw_random.h +++ b/include/linux/hw_random.h @@ -50,6 +50,7 @@ struct hwrng { struct list_head list; struct kref ref; struct completion cleanup_done; + bool isprng; }; What do you think about those two solutions ?