From patchwork Thu Sep 15 00:22:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven van Ashbrook X-Patchwork-Id: 606317 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CFF67C6FA82 for ; Thu, 15 Sep 2022 00:23:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229885AbiIOAXC (ORCPT ); Wed, 14 Sep 2022 20:23:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54034 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229586AbiIOAXB (ORCPT ); Wed, 14 Sep 2022 20:23:01 -0400 Received: from mail-qt1-x82a.google.com (mail-qt1-x82a.google.com [IPv6:2607:f8b0:4864:20::82a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1BF148052F for ; Wed, 14 Sep 2022 17:23:00 -0700 (PDT) Received: by mail-qt1-x82a.google.com with SMTP id c11so12454923qtw.8 for ; Wed, 14 Sep 2022 17:23:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date; bh=eRiMKscQqGLWHiLc+7+i9xWjQkO+eAs1NiZ4jOvGS0U=; b=DDKycVVoBoWbCM3nQMDQEe8g1JNrOsjXVyfZHCebseS4+afsgAkQYMIPCaGK1izdDM jObnH0Dtsq8TtMdYHeoe8jd1zKZZhyuyg1q9B5mKnPFlbJJRuorQ+GIhMZ0dQtXWu7TW j9Ta3r8XkKQ5zPasxQulLbgCtgRgUSGETaydY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date; bh=eRiMKscQqGLWHiLc+7+i9xWjQkO+eAs1NiZ4jOvGS0U=; b=vxdAWpADFB2x1xKWNyzfv8JPDSAVDp0VpqLQbBqu9+roi+0VJCfoBXD8IjRXGVbttt 2oP08BtwSuoD7GUb3FQaKTG7Ycu0Uo+w4J7ySSiV+OTG4Btw9YCTKRjZ8d7tD2rnCSko sFunFh4HADR0Kyrb6pdh+LMgojgMkNOCPLE84tTA/TQ02utA74/O92/0VAEPxLogatNQ yooDT0T8Piu/YpTscj8TC73X7VDPu94whF72ELJwmbrLxj+rIoWXn730yHhLR+BNl4d+ choTM542+9RTe7mN8cxHup2pyGc/ugz+gd6rBAFBypusmD+C5pi53F07JgI6tuPRj40z 8esQ== X-Gm-Message-State: ACgBeo1/Y+5pypr2Wcgdm7cR4PZNoaz+/8tc6yQIZRW8n/9EVPY8Y3go LB6B2WtmxeqyZZfWXaCyMTjboA== X-Google-Smtp-Source: AA6agR5n0qg3MJMUDrj6cUWrw4bfwjRO+f7dxAy+UMb7nJNVnhtqjMVgK7ybK2zcEHxqmw36epXNPg== X-Received: by 2002:a05:622a:38f:b0:35b:b8e7:82ef with SMTP id j15-20020a05622a038f00b0035bb8e782efmr13021242qtx.647.1663201379203; Wed, 14 Sep 2022 17:22:59 -0700 (PDT) Received: from trappist.c.googlers.com.com (128.174.85.34.bc.googleusercontent.com. [34.85.174.128]) by smtp.gmail.com with ESMTPSA id m5-20020ac86885000000b0034367ec57ecsm2377611qtq.9.2022.09.14.17.22.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Sep 2022 17:22:58 -0700 (PDT) From: Sven van Ashbrook To: LKML , Herbert Xu , Dominik Brodowski , Olivia Mackall Cc: Alex Levin , Andrey Pronin , Stephen Boyd , Rajat Jain , Sven van Ashbrook , Eric Biggers , "Jason A. Donenfeld" , Petr Mladek , Sebastian Andrzej Siewior , Theodore Ts'o , linux-crypto@vger.kernel.org Subject: [PATCH v2 1/2] random: move add_hwgenerator_randomness()'s wait outside function Date: Thu, 15 Sep 2022 00:22:53 +0000 Message-Id: <20220915002235.v2.1.I7c0a79e9b3c52584f5b637fde5f1d6f807605806@changeid> X-Mailer: git-send-email 2.37.2.789.g6183377224-goog MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org add_hwgenerator_randomness() currently blocks until more entropy is needed. Move the blocking wait out of the function to the caller, by letting the function return the number of jiffies needed to block. This is done to prepare the function's sole kernel caller from a kthread to self-rearming delayed_work. Signed-off-by: Sven van Ashbrook --- Changes in v2: - justify patch as a preparation for next patch drivers/char/hw_random/core.c | 7 +++++-- drivers/char/random.c | 13 ++++++------- include/linux/random.h | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 16f227b995e8..3675122c6cce 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -491,6 +491,7 @@ static int __init register_miscdev(void) static int hwrng_fillfn(void *unused) { size_t entropy, entropy_credit = 0; /* in 1/1024 of a bit */ + unsigned long delay; long rc; while (!kthread_should_stop()) { @@ -526,8 +527,10 @@ static int hwrng_fillfn(void *unused) entropy_credit = entropy; /* Outside lock, sure, but y'know: randomness. */ - add_hwgenerator_randomness((void *)rng_fillbuf, rc, - entropy >> 10); + delay = add_hwgenerator_randomness((void *)rng_fillbuf, rc, + entropy >> 10); + if (delay > 0) + schedule_timeout_interruptible(delay); } hwrng_fill = NULL; return 0; diff --git a/drivers/char/random.c b/drivers/char/random.c index 79d7d4e4e582..5dc949298f92 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -686,7 +686,7 @@ static void __cold _credit_init_bits(size_t bits) * the above entropy accumulation routines: * * void add_device_randomness(const void *buf, size_t len); - * void add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy); + * unsigned long add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy); * void add_bootloader_randomness(const void *buf, size_t len); * void add_vmfork_randomness(const void *unique_vm_id, size_t len); * void add_interrupt_randomness(int irq); @@ -702,8 +702,8 @@ static void __cold _credit_init_bits(size_t bits) * available to them (particularly common in the embedded world). * * add_hwgenerator_randomness() is for true hardware RNGs, and will credit - * entropy as specified by the caller. If the entropy pool is full it will - * block until more entropy is needed. + * entropy as specified by the caller. Returns time delay in jiffies until + * more entropy is needed. * * add_bootloader_randomness() is called by bootloader drivers, such as EFI * and device tree, and credits its input depending on whether or not the @@ -857,10 +857,10 @@ EXPORT_SYMBOL(add_device_randomness); /* * Interface for in-kernel drivers of true hardware RNGs. - * Those devices may produce endless random bits and will be throttled + * Those devices may produce endless random bits and should be throttled * when our pool is full. */ -void add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy) +unsigned long add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy) { mix_pool_bytes(buf, len); credit_init_bits(entropy); @@ -869,8 +869,7 @@ void add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy) * Throttle writing to once every CRNG_RESEED_INTERVAL, unless * we're not yet initialized. */ - if (!kthread_should_stop() && crng_ready()) - schedule_timeout_interruptible(CRNG_RESEED_INTERVAL); + return crng_ready() ? CRNG_RESEED_INTERVAL : 0; } EXPORT_SYMBOL_GPL(add_hwgenerator_randomness); diff --git a/include/linux/random.h b/include/linux/random.h index 3fec206487f6..6608b0fb4402 100644 --- a/include/linux/random.h +++ b/include/linux/random.h @@ -17,7 +17,7 @@ void __init add_bootloader_randomness(const void *buf, size_t len); void add_input_randomness(unsigned int type, unsigned int code, unsigned int value) __latent_entropy; void add_interrupt_randomness(int irq) __latent_entropy; -void add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy); +unsigned long add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy); #if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__) static inline void add_latent_entropy(void) From patchwork Thu Sep 15 00:22:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven van Ashbrook X-Patchwork-Id: 606554 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 753BAC6FA89 for ; Thu, 15 Sep 2022 00:23:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230063AbiIOAXE (ORCPT ); Wed, 14 Sep 2022 20:23:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54064 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230052AbiIOAXD (ORCPT ); Wed, 14 Sep 2022 20:23:03 -0400 Received: from mail-qt1-x834.google.com (mail-qt1-x834.google.com [IPv6:2607:f8b0:4864:20::834]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D829F8052F for ; Wed, 14 Sep 2022 17:23:01 -0700 (PDT) Received: by mail-qt1-x834.google.com with SMTP id b23so7551605qtr.13 for ; Wed, 14 Sep 2022 17:23:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date; bh=BnCdr1U/em5NpPCWgG/aOnm65mlthSVN6z+DfX/ZHto=; b=PvdF1V9kUYUb0PK/HjhbxSbYF5T2tMtxcCB9+M8nMpqPrPFwz6ALyE48DIfGieOqGp v1kouRf80IOAxzJlugoAihdagG9tPWp1IZgC0XUCaL3A71eoAQT6gahqL6s4EUWJHlei TMVft8Orr1iCRfI+kYSOfjdb/XA+mjsEwTEgA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date; bh=BnCdr1U/em5NpPCWgG/aOnm65mlthSVN6z+DfX/ZHto=; b=3JPF05XAT4PrJXKYH+iRAMMHEP19y6XipyQuBTiKZpQjsFXvXhpLPln8kQc+h87kW2 rsoSdTSCbdZYIKhBE8J+2L8n5lfQXyXPGLeIApzM8iIPmjNYy9RLAgKuw0HJWYyRHHtd wN3M97GN5VL8yRJGIUGU1je1IS4Shs6wqwzu9yE3hpJ1qHNlfHSQrN1WO9osHvdsTqnD WtPYj3mtT/aRPBdwTfQH7pPdnxv4Jz3z8m+y3P45uSEv6+UlSjyahq1JuQZ/Tah3zs9Z y3UXFLETPhE31pzzGyD6res7haa1SV1tf6w9suLfb5zDyOMPj/juz7E/lVBEr8tHSvOV 3Tsg== X-Gm-Message-State: ACgBeo2wg11TQyzb6a6EgSBG6ClK4y10ckj/XAjPpHPuKnPnGhjpaHkU AwDzYKJQMT8MlSsjvRfL8BPC8Q== X-Google-Smtp-Source: AA6agR6PqdDOTaxrQgkWjRZA7p0lkg0Nc3A7gVBubgRSifUTqelkJp2YUvAkVhTnMNjRviqGusTccw== X-Received: by 2002:a05:622a:290:b0:35b:bc26:d98c with SMTP id z16-20020a05622a029000b0035bbc26d98cmr10713300qtw.489.1663201381041; Wed, 14 Sep 2022 17:23:01 -0700 (PDT) Received: from trappist.c.googlers.com.com (128.174.85.34.bc.googleusercontent.com. [34.85.174.128]) by smtp.gmail.com with ESMTPSA id m5-20020ac86885000000b0034367ec57ecsm2377611qtq.9.2022.09.14.17.23.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Sep 2022 17:23:00 -0700 (PDT) From: Sven van Ashbrook To: LKML , Herbert Xu , Dominik Brodowski , Olivia Mackall Cc: Alex Levin , Andrey Pronin , Stephen Boyd , Rajat Jain , Sven van Ashbrook , Eric Biggers , "Jason A. Donenfeld" , linux-crypto@vger.kernel.org Subject: [PATCH v2 2/2] hwrng: core: fix suspend/resume race condition Date: Thu, 15 Sep 2022 00:22:54 +0000 Message-Id: <20220915002235.v2.2.I8db2637ed3b9abc3961f3bc41ec25fa8b8ae6624@changeid> X-Mailer: git-send-email 2.37.2.789.g6183377224-goog In-Reply-To: <20220915002235.v2.1.I7c0a79e9b3c52584f5b637fde5f1d6f807605806@changeid> References: <20220915002235.v2.1.I7c0a79e9b3c52584f5b637fde5f1d6f807605806@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org The hwrng fill function runs as a normal kthread. This thread doesn't get frozen by the PM, i.e. it will keep running during, or in, system suspend. It may call the client driver's data_present()/data_read() functions during, or in, suspend; which may generate errors or warnings. For example, if the client driver uses an i2c bus, the following warning may be intermittently generated: i2c: Transfer while suspended Fix by converting the delay polled kthread into an ordered work queue running a single, self-rearming delayed_work. Make the workqueue WQ_FREEZABLE, so the PM will drain any work items before going into suspend. This prevents client drivers from being accessed during, or in, suspend. Tested on a Chromebook containing an cr50 tpm over i2c. The test consists of 31000 suspend/resume cycles. Occasional "i2c: Transfer while suspended" warnings are seen. After applying this patch, these warnings disappear. This patch also does not appear to cause any regressions on the ChromeOS test queues. Signed-off-by: Sven van Ashbrook --- Changes in v2: - refreshed patch set against latest tree drivers/char/hw_random/core.c | 95 +++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 3675122c6cce..ee85ca97d215 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include #include @@ -28,14 +28,17 @@ #define RNG_MODULE_NAME "hw_random" -static struct hwrng *current_rng; /* the current rng has been explicitly chosen by user via sysfs */ static int cur_rng_set_by_user; -static struct task_struct *hwrng_fill; +static struct workqueue_struct *hwrng_wq; +static struct delayed_work hwrng_fill_dwork; +static size_t entropy_credit; +/* Protects rng_list, current_rng, is_hwrng_wq_running */ +static DEFINE_MUTEX(rng_mutex); /* list of registered rngs */ static LIST_HEAD(rng_list); -/* Protects rng_list and current_rng */ -static DEFINE_MUTEX(rng_mutex); +static struct hwrng *current_rng; +static bool is_hwrng_wq_running; /* Protects rng read functions, data_avail, rng_buffer and rng_fillbuf */ static DEFINE_MUTEX(reading_mutex); static int data_avail; @@ -488,37 +491,29 @@ static int __init register_miscdev(void) return misc_register(&rng_miscdev); } -static int hwrng_fillfn(void *unused) +static void hwrng_fillfn(struct work_struct *unused) { - size_t entropy, entropy_credit = 0; /* in 1/1024 of a bit */ + unsigned short quality; unsigned long delay; + struct hwrng *rng; + size_t entropy; /* in 1/1024 of a bit */ long rc; - while (!kthread_should_stop()) { - unsigned short quality; - struct hwrng *rng; - - rng = get_current_rng(); - if (IS_ERR(rng) || !rng) - break; - mutex_lock(&reading_mutex); - rc = rng_get_data(rng, rng_fillbuf, - rng_buffer_size(), 1); - if (current_quality != rng->quality) - rng->quality = current_quality; /* obsolete */ - quality = rng->quality; - mutex_unlock(&reading_mutex); - put_rng(rng); - - if (!quality) - break; + rng = get_current_rng(); + if (IS_ERR(rng) || !rng) + return; + mutex_lock(&reading_mutex); + rc = rng_get_data(rng, rng_fillbuf, rng_buffer_size(), 1); + if (current_quality != rng->quality) + rng->quality = current_quality; /* obsolete */ + quality = rng->quality; + mutex_unlock(&reading_mutex); + put_rng(rng); - if (rc <= 0) { - pr_warn("hwrng: no data available\n"); - msleep_interruptible(10000); - continue; - } + if (!quality) + return; + if (rc > 0) { /* If we cannot credit at least one bit of entropy, * keep track of the remainder for the next iteration */ @@ -529,11 +524,11 @@ static int hwrng_fillfn(void *unused) /* Outside lock, sure, but y'know: randomness. */ delay = add_hwgenerator_randomness((void *)rng_fillbuf, rc, entropy >> 10); - if (delay > 0) - schedule_timeout_interruptible(delay); + } else { + pr_warn("hwrng: no data available\n"); + delay = 10 * HZ; } - hwrng_fill = NULL; - return 0; + mod_delayed_work(hwrng_wq, &hwrng_fill_dwork, delay); } static void hwrng_manage_rngd(struct hwrng *rng) @@ -541,14 +536,12 @@ static void hwrng_manage_rngd(struct hwrng *rng) if (WARN_ON(!mutex_is_locked(&rng_mutex))) return; - if (rng->quality == 0 && hwrng_fill) - kthread_stop(hwrng_fill); - if (rng->quality > 0 && !hwrng_fill) { - hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng"); - if (IS_ERR(hwrng_fill)) { - pr_err("hwrng_fill thread creation failed\n"); - hwrng_fill = NULL; - } + if (rng->quality == 0 && is_hwrng_wq_running) { + cancel_delayed_work(&hwrng_fill_dwork); + is_hwrng_wq_running = false; + } else if (rng->quality > 0 && !is_hwrng_wq_running) { + mod_delayed_work(hwrng_wq, &hwrng_fill_dwork, 0); + is_hwrng_wq_running = true; } } @@ -631,8 +624,7 @@ void hwrng_unregister(struct hwrng *rng) new_rng = get_current_rng_nolock(); if (list_empty(&rng_list)) { mutex_unlock(&rng_mutex); - if (hwrng_fill) - kthread_stop(hwrng_fill); + cancel_delayed_work_sync(&hwrng_fill_dwork); } else mutex_unlock(&rng_mutex); @@ -703,17 +695,32 @@ static int __init hwrng_modinit(void) return -ENOMEM; } + /* ordered wq to mimic delay-polled kthread behaviour */ + hwrng_wq = alloc_ordered_workqueue("hwrng", + WQ_FREEZABLE | /* prevent work from running during suspend/resume */ + WQ_MEM_RECLAIM /* client drivers may need memory reclaim */ + ); + if (!hwrng_wq) { + kfree(rng_fillbuf); + kfree(rng_buffer); + return -ENOMEM; + } + ret = register_miscdev(); if (ret) { + destroy_workqueue(hwrng_wq); kfree(rng_fillbuf); kfree(rng_buffer); } + INIT_DELAYED_WORK(&hwrng_fill_dwork, hwrng_fillfn); + return ret; } static void __exit hwrng_modexit(void) { + destroy_workqueue(hwrng_wq); mutex_lock(&rng_mutex); BUG_ON(current_rng); kfree(rng_buffer);