Re: [PATCH 3/4] crypto: qcom-rng - Remove crypto_rng interface

[Neeraj Soni]

On 5/30/2026 7:33 AM, Eric Biggers wrote:
> qcom-rng.c exposes the same hardware through two completely separate
> interfaces, crypto_rng and hwrng.  However, the implementation of this
> is buggy because it permits generation operations from these interfaces
> to run concurrently with each other, accessing the same registers.  That
> is, qcom_rng_generate() synchronizes with itself but not with
> qcom_hwrng_read().  This results in potential repetition of output from
> the RNG, output of non-random values, etc.
> 
> Fortunately, there's actually no point in hardware RNG drivers
> implementing the crypto_rng interface.  It's not actually used by
> anything besides the "rng" algorithm type of AF_ALG, which in turn is
> not actually used in practice.  Other crypto_rng hardware drivers are

How it was established that there are no active users/clints for qcom-rng
using crypto_rng interface? If there is no concrete way to do then this
patch breaks backward compatibility.

> likewise being phased out, leaving just the hwrng support.
> 
> Thus, remove it to simplify the code and avoid conflict (and confusion)
> with the hwrng interface which is the one that actually matters.
> 
> Note that while this means the driver stops supporting "qcom,prng" and
> "qcom,prng-ee", it didn't do anything useful on SoCs with those anyway.
> 
> Fixes: f29cd5bb64c2 ("crypto: qcom-rng - Add hw_random interface support")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> ---
>  drivers/crypto/Kconfig    |   1 -
>  drivers/crypto/qcom-rng.c | 175 ++------------------------------------
>  2 files changed, 9 insertions(+), 167 deletions(-)
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 3449b3c9c6ad..a12cd677467b 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -658,11 +658,10 @@ config CRYPTO_DEV_QCE_SW_MAX_LEN
>  
>  config CRYPTO_DEV_QCOM_RNG
>  	tristate "Qualcomm Random Number Generator Driver"
>  	depends on ARCH_QCOM || COMPILE_TEST
>  	depends on HW_RANDOM
> -	select CRYPTO_RNG
>  	help
>  	  This driver provides support for the Random Number
>  	  Generator hardware found on Qualcomm SoCs.
>  
>  	  To compile this driver as a module, choose M here. The
> diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
> index b7f3b9695dac..48b605687b28 100644
> --- a/drivers/crypto/qcom-rng.c
> +++ b/drivers/crypto/qcom-rng.c
> @@ -1,14 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2017-18 Linaro Limited
>  //
>  // Based on msm-rng.c and downstream driver
>  
> -#include <crypto/internal/rng.h>
> -#include <linux/acpi.h>
>  #include <linux/clk.h>
> -#include <linux/crypto.h>
>  #include <linux/hw_random.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> @@ -30,28 +27,15 @@
>  #define WORD_SZ			4
>  
>  #define QCOM_TRNG_QUALITY	1024
>  
>  struct qcom_rng {
> -	struct mutex lock;
>  	void __iomem *base;
>  	struct clk *clk;
>  	struct hwrng hwrng;
> -	struct qcom_rng_match_data *match_data;
>  };
>  
> -struct qcom_rng_ctx {
> -	struct qcom_rng *rng;
> -};
> -
> -struct qcom_rng_match_data {
> -	bool skip_init;
> -	bool hwrng_support;
> -};
> -
> -static struct qcom_rng *qcom_rng_dev;
> -
>  static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max)
>  {
>  	unsigned int currsize = 0;
>  	u32 val;
>  	int ret;
> @@ -77,41 +61,10 @@ static int qcom_rng_read(struct qcom_rng *rng, u8 *data, unsigned int max)
>  	} while (currsize < max);
>  
>  	return currsize;
>  }
>  
> -static int qcom_rng_generate(struct crypto_rng *tfm,
> -			     const u8 *src, unsigned int slen,
> -			     u8 *dstn, unsigned int dlen)
> -{
> -	struct qcom_rng_ctx *ctx = crypto_rng_ctx(tfm);
> -	struct qcom_rng *rng = ctx->rng;
> -	int ret;
> -
> -	ret = clk_prepare_enable(rng->clk);
> -	if (ret)
> -		return ret;
> -
> -	mutex_lock(&rng->lock);
> -
> -	ret = qcom_rng_read(rng, dstn, dlen);
> -
> -	mutex_unlock(&rng->lock);
> -	clk_disable_unprepare(rng->clk);
> -
> -	if (ret >= 0)
> -		ret = 0;
> -
> -	return ret;
> -}
> -
> -static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed,
> -			 unsigned int slen)
> -{
> -	return 0;
> -}
> -
>  static int qcom_hwrng_init(struct hwrng *hwrng)
>  {
>  	struct qcom_rng *qrng = container_of(hwrng, struct qcom_rng, hwrng);
>  
>  	return clk_prepare_enable(qrng->clk);
> @@ -129,159 +82,49 @@ static void qcom_hwrng_cleanup(struct hwrng *hwrng)
>  	struct qcom_rng *qrng = container_of(hwrng, struct qcom_rng, hwrng);
>  
>  	clk_disable_unprepare(qrng->clk);
>  }
>  
> -static int qcom_rng_enable(struct qcom_rng *rng)
> -{
> -	u32 val;
> -	int ret;
> -
> -	ret = clk_prepare_enable(rng->clk);
> -	if (ret)
> -		return ret;
> -
> -	/* Enable PRNG only if it is not already enabled */
> -	val = readl_relaxed(rng->base + PRNG_CONFIG);
> -	if (val & PRNG_CONFIG_HW_ENABLE)
> -		goto already_enabled;
> -
> -	val = readl_relaxed(rng->base + PRNG_LFSR_CFG);
> -	val &= ~PRNG_LFSR_CFG_MASK;
> -	val |= PRNG_LFSR_CFG_CLOCKS;
> -	writel(val, rng->base + PRNG_LFSR_CFG);
> -
> -	val = readl_relaxed(rng->base + PRNG_CONFIG);
> -	val |= PRNG_CONFIG_HW_ENABLE;
> -	writel(val, rng->base + PRNG_CONFIG);
> -
> -already_enabled:
> -	clk_disable_unprepare(rng->clk);
> -
> -	return 0;
> -}
> -
> -static int qcom_rng_init(struct crypto_tfm *tfm)
> -{
> -	struct qcom_rng_ctx *ctx = crypto_tfm_ctx(tfm);
> -
> -	ctx->rng = qcom_rng_dev;
> -
> -	if (!ctx->rng->match_data->skip_init)
> -		return qcom_rng_enable(ctx->rng);
> -
> -	return 0;
> -}
> -
> -static struct rng_alg qcom_rng_alg = {
> -	.generate	= qcom_rng_generate,
> -	.seed		= qcom_rng_seed,
> -	.seedsize	= 0,
> -	.base		= {
> -		.cra_name		= "stdrng",
> -		.cra_driver_name	= "qcom-rng",
> -		.cra_flags		= CRYPTO_ALG_TYPE_RNG,
> -		.cra_priority		= 300,
> -		.cra_ctxsize		= sizeof(struct qcom_rng_ctx),
> -		.cra_module		= THIS_MODULE,
> -		.cra_init		= qcom_rng_init,
> -	}
> -};
> -
>  static int qcom_rng_probe(struct platform_device *pdev)
>  {
>  	struct qcom_rng *rng;
>  	int ret;
>  
>  	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
>  	if (!rng)
>  		return -ENOMEM;
>  
> -	platform_set_drvdata(pdev, rng);
> -	mutex_init(&rng->lock);
> -
>  	rng->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(rng->base))
>  		return PTR_ERR(rng->base);
>  
>  	rng->clk = devm_clk_get_optional(&pdev->dev, "core");
>  	if (IS_ERR(rng->clk))
>  		return PTR_ERR(rng->clk);
>  
> -	rng->match_data = (struct qcom_rng_match_data *)device_get_match_data(&pdev->dev);
> -
> -	qcom_rng_dev = rng;
> -	ret = crypto_register_rng(&qcom_rng_alg);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Register crypto rng failed: %d\n", ret);
> -		qcom_rng_dev = NULL;
> -		return ret;
> -	}
> -
> -	if (rng->match_data->hwrng_support) {
> -		rng->hwrng.name = "qcom_hwrng";
> -		rng->hwrng.init = qcom_hwrng_init;
> -		rng->hwrng.read = qcom_hwrng_read;
> -		rng->hwrng.cleanup = qcom_hwrng_cleanup;
> -		rng->hwrng.quality = QCOM_TRNG_QUALITY;
> -		ret = devm_hwrng_register(&pdev->dev, &rng->hwrng);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Register hwrng failed: %d\n", ret);
> -			qcom_rng_dev = NULL;
> -			goto fail;
> -		}
> -	}
> -
> -	return ret;
> -fail:
> -	crypto_unregister_rng(&qcom_rng_alg);
> +	rng->hwrng.name = "qcom_hwrng";
> +	rng->hwrng.init = qcom_hwrng_init;
> +	rng->hwrng.read = qcom_hwrng_read;
> +	rng->hwrng.cleanup = qcom_hwrng_cleanup;
> +	rng->hwrng.quality = QCOM_TRNG_QUALITY;
> +	ret = devm_hwrng_register(&pdev->dev, &rng->hwrng);
> +	if (ret)
> +		dev_err(&pdev->dev, "Register hwrng failed: %d\n", ret);
>  	return ret;
>  }
>  
> -static void qcom_rng_remove(struct platform_device *pdev)
> -{
> -	crypto_unregister_rng(&qcom_rng_alg);
> -
> -	qcom_rng_dev = NULL;
> -}
> -
> -static struct qcom_rng_match_data qcom_prng_match_data = {
> -	.skip_init = false,
> -	.hwrng_support = false,
> -};
> -
> -static struct qcom_rng_match_data qcom_prng_ee_match_data = {
> -	.skip_init = true,
> -	.hwrng_support = false,
> -};
> -
> -static struct qcom_rng_match_data qcom_trng_match_data = {
> -	.skip_init = true,
> -	.hwrng_support = true,
> -};
> -
> -static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = {
> -	{ .id = "QCOM8160", .driver_data = (kernel_ulong_t)&qcom_prng_ee_match_data },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(acpi, qcom_rng_acpi_match);
> -
>  static const struct of_device_id __maybe_unused qcom_rng_of_match[] = {
> -	{ .compatible = "qcom,prng", .data = &qcom_prng_match_data },
> -	{ .compatible = "qcom,prng-ee", .data = &qcom_prng_ee_match_data },
> -	{ .compatible = "qcom,trng", .data = &qcom_trng_match_data },
> +	{ .compatible = "qcom,trng" },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, qcom_rng_of_match);
>  
>  static struct platform_driver qcom_rng_driver = {
>  	.probe = qcom_rng_probe,
> -	.remove =  qcom_rng_remove,
>  	.driver = {
>  		.name = KBUILD_MODNAME,
>  		.of_match_table = qcom_rng_of_match,
> -		.acpi_match_table = ACPI_PTR(qcom_rng_acpi_match),
>  	}
>  };
>  module_platform_driver(qcom_rng_driver);
>  
>  MODULE_ALIAS("platform:" KBUILD_MODNAME);
> 

Regards,
Neeraj