2023-06-16 08:41:43

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 0/2] hwrng: st - fix potential race condition

Fix a potential race condition in the st-rng driver. There's a short timeframe
where the driver is still registered but its peripheral clock is disabled.

Add support for compile-testing the driver. I do not have any hardware that
supports st-rng.

Martin Kaiser (2):
hwrng: st - support compile-testing
hwrng: st - keep clock enabled while hwrng is registered

drivers/char/hw_random/Kconfig | 2 +-
drivers/char/hw_random/st-rng.c | 18 +-----------------
2 files changed, 2 insertions(+), 18 deletions(-)

--
2.30.2



2023-06-16 08:41:45

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 1/2] hwrng: st - support compile-testing

Allow compile-testing the st-rng driver if we're not running on an ST
chipset.

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/char/hw_random/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index baefa2e0edbc..e0b3786ca51b 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -348,7 +348,7 @@ config HW_RANDOM_HISTB

config HW_RANDOM_ST
tristate "ST Microelectronics HW Random Number Generator support"
- depends on HW_RANDOM && ARCH_STI
+ depends on HW_RANDOM && (ARCH_STI || COMPILE_TEST)
help
This driver provides kernel-side support for the Random Number
Generator hardware found on STi series of SoCs.
--
2.30.2


2023-06-16 08:41:58

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 2/2] hwrng: st - keep clock enabled while hwrng is registered

The st-rng driver uses devres to register itself with the hwrng core,
the driver will be unregistered from hwrng when its device goes out of
scope. This happens after the driver's remove function is called.

However, st-rng's clock is disabled in the remove function. There's a
short timeframe where st-rng is still registered with the hwrng core
although its clock is disabled. I suppose the clock must be active to
access the hardware and serve requests from the hwrng core.

Switch to devm_clk_get_enabled and let devres handle both clock and hwrng
registration. This avoids the race condition.

Fixes: 3e75241be808 ("hwrng: drivers - Use device-managed registration API")
Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/char/hw_random/st-rng.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/char/hw_random/st-rng.c b/drivers/char/hw_random/st-rng.c
index 15ba1e6fae4d..7a4e439d34d7 100644
--- a/drivers/char/hw_random/st-rng.c
+++ b/drivers/char/hw_random/st-rng.c
@@ -42,7 +42,6 @@

struct st_rng_data {
void __iomem *base;
- struct clk *clk;
struct hwrng ops;
};

@@ -85,19 +84,14 @@ static int st_rng_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);

- clk = devm_clk_get(&pdev->dev, NULL);
+ clk = devm_clk_get_enabled(&pdev->dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);

- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
ddata->ops.priv = (unsigned long)ddata;
ddata->ops.read = st_rng_read;
ddata->ops.name = pdev->name;
ddata->base = base;
- ddata->clk = clk;

dev_set_drvdata(&pdev->dev, ddata);

@@ -113,15 +107,6 @@ static int st_rng_probe(struct platform_device *pdev)
return 0;
}

-static int st_rng_remove(struct platform_device *pdev)
-{
- struct st_rng_data *ddata = dev_get_drvdata(&pdev->dev);
-
- clk_disable_unprepare(ddata->clk);
-
- return 0;
-}
-
static const struct of_device_id st_rng_match[] __maybe_unused = {
{ .compatible = "st,rng" },
{},
@@ -134,7 +119,6 @@ static struct platform_driver st_rng_driver = {
.of_match_table = of_match_ptr(st_rng_match),
},
.probe = st_rng_probe,
- .remove = st_rng_remove
};

module_platform_driver(st_rng_driver);
--
2.30.2


2023-06-16 09:04:03

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH v2 0/2] hwrng: st - fix potential race condition

Fix a potential race condition in the st-rng driver. There's a short timeframe
where the driver is still registered but its peripheral clock is disabled.

Add support for compile-testing the driver. I do not have any hardware that
supports st-rng.

v2: Remove some more obsolete code and rephrase the explanation. The point is
that devres does now disable the clock.

Martin Kaiser (2):
hwrng: st - support compile-testing
hwrng: st - keep clock enabled while hwrng is registered

drivers/char/hw_random/Kconfig | 2 +-
drivers/char/hw_random/st-rng.c | 21 +--------------------
2 files changed, 2 insertions(+), 21 deletions(-)

--
2.30.2


2023-06-16 09:04:13

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH v2 2/2] hwrng: st - keep clock enabled while hwrng is registered

The st-rng driver uses devres to register itself with the hwrng core,
the driver will be unregistered from hwrng when its device goes out of
scope. This happens after the driver's remove function is called.

However, st-rng's clock is disabled in the remove function. There's a
short timeframe where st-rng is still registered with the hwrng core
although its clock is disabled. I suppose the clock must be active to
access the hardware and serve requests from the hwrng core.

Switch to devm_clk_get_enabled and let devres disable the clock and
unregister the hwrng. This avoids the race condition.

Fixes: 3e75241be808 ("hwrng: drivers - Use device-managed registration API")
Signed-off-by: Martin Kaiser <[email protected]>
---
v2:
- don't unprepare the clock if devm_hwrng_register fails
- don't set driver data, it was used only in the remove function that's
now gone
- rephrase the commit message, the clock is now disabled by devres

drivers/char/hw_random/st-rng.c | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/char/hw_random/st-rng.c b/drivers/char/hw_random/st-rng.c
index 15ba1e6fae4d..6e9dfac9fc9f 100644
--- a/drivers/char/hw_random/st-rng.c
+++ b/drivers/char/hw_random/st-rng.c
@@ -42,7 +42,6 @@

struct st_rng_data {
void __iomem *base;
- struct clk *clk;
struct hwrng ops;
};

@@ -85,26 +84,18 @@ static int st_rng_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);

- clk = devm_clk_get(&pdev->dev, NULL);
+ clk = devm_clk_get_enabled(&pdev->dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);

- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
ddata->ops.priv = (unsigned long)ddata;
ddata->ops.read = st_rng_read;
ddata->ops.name = pdev->name;
ddata->base = base;
- ddata->clk = clk;
-
- dev_set_drvdata(&pdev->dev, ddata);

ret = devm_hwrng_register(&pdev->dev, &ddata->ops);
if (ret) {
dev_err(&pdev->dev, "Failed to register HW RNG\n");
- clk_disable_unprepare(clk);
return ret;
}

@@ -113,15 +104,6 @@ static int st_rng_probe(struct platform_device *pdev)
return 0;
}

-static int st_rng_remove(struct platform_device *pdev)
-{
- struct st_rng_data *ddata = dev_get_drvdata(&pdev->dev);
-
- clk_disable_unprepare(ddata->clk);
-
- return 0;
-}
-
static const struct of_device_id st_rng_match[] __maybe_unused = {
{ .compatible = "st,rng" },
{},
@@ -134,7 +116,6 @@ static struct platform_driver st_rng_driver = {
.of_match_table = of_match_ptr(st_rng_match),
},
.probe = st_rng_probe,
- .remove = st_rng_remove
};

module_platform_driver(st_rng_driver);
--
2.30.2


2023-06-16 09:04:30

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH v2 1/2] hwrng: st - support compile-testing

Allow compile-testing the st-rng driver if we're not running on an ST
chipset.

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/char/hw_random/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index baefa2e0edbc..e0b3786ca51b 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -348,7 +348,7 @@ config HW_RANDOM_HISTB

config HW_RANDOM_ST
tristate "ST Microelectronics HW Random Number Generator support"
- depends on HW_RANDOM && ARCH_STI
+ depends on HW_RANDOM && (ARCH_STI || COMPILE_TEST)
help
This driver provides kernel-side support for the Random Number
Generator hardware found on STi series of SoCs.
--
2.30.2


2023-06-23 08:29:15

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] hwrng: st - fix potential race condition

On Fri, Jun 16, 2023 at 09:58:11AM +0100, Martin Kaiser wrote:
> Fix a potential race condition in the st-rng driver. There's a short timeframe
> where the driver is still registered but its peripheral clock is disabled.
>
> Add support for compile-testing the driver. I do not have any hardware that
> supports st-rng.
>
> v2: Remove some more obsolete code and rephrase the explanation. The point is
> that devres does now disable the clock.
>
> Martin Kaiser (2):
> hwrng: st - support compile-testing
> hwrng: st - keep clock enabled while hwrng is registered
>
> drivers/char/hw_random/Kconfig | 2 +-
> drivers/char/hw_random/st-rng.c | 21 +--------------------
> 2 files changed, 2 insertions(+), 21 deletions(-)
>
> --
> 2.30.2

All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt