[PATCH v5 1/2] dt-bindings: rng: timeriomem_rng: add reg-io-width and mask properties

[PATCH v5 1/2] dt-bindings: rng: timeriomem_rng: add reg-io-width and mask properties

[Jad Keskes]

Add optional reg-io-width (1, 2, or 4 bytes) and mask properties.
reg-io-width selects the bus access size.  mask is ANDed with the raw
register value to allow only the entropy-bearing bits through.

Update the example to show a 1-byte configuration.

Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
v5:
- Fix description to describe hardware, not the binding
- Drop SPDX dual-license change
- Merge examples into one
v4: Initial version with reg-io-width (replaced custom width from v3)
v3: Changed from custom width to reg-io-width per dt-bindings convention
v2: Split DT binding and driver into separate patches
v1: Initial submission
---
 .../bindings/rng/timeriomem_rng.yaml          | 50 ++++++++++++++-----
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
index 4754174e9849..7f0068f785b7 100644
--- a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
+++ b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
@@ -4,7 +4,11 @@
 $id: http://devicetree.org/schemas/rng/timeriomem_rng.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: TimerIO Random Number Generator
+title: Timer IOMEM hardware random number generator
+
+description: |
+  A device that provides random data via a single memory-mapped IO register.
+  A new value becomes available at a fixed interval.
 
 maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
@@ -13,29 +17,47 @@ properties:
   compatible:
     const: timeriomem_rng
 
+  reg:
+    maxItems: 1
+    description:
+      Base address to sample from.  Must be aligned to the configured access
+      width (1, 2, or 4 bytes) and at least that wide.
+
   period:
     $ref: /schemas/types.yaml#/definitions/uint32
-    description: wait time in microseconds to use between samples
+    description:
+      Interval in microseconds between random value updates.
 
   quality:
     $ref: /schemas/types.yaml#/definitions/uint32
     default: 0
     description:
-      Estimated number of bits of true entropy per 1024 bits read from the rng.
-      Defaults to zero which causes the kernel's default quality to be used
-      instead.  Note that the default quality is usually zero which disables
-      using this rng to automatically fill the kernel's entropy pool.
+      Estimated number of bits of true entropy per 1024 bits read from the
+      device.  Defaults to zero which causes the kernel's default quality to
+      be used instead.  Note that the default quality is usually zero which
+      disables using this RNG to automatically fill the kernel's entropy
+      pool.
 
-  reg:
-    maxItems: 1
+  reg-io-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 4
+    enum: [1, 2, 4]
     description:
-      Base address to sample from. Currently 'reg' must be at least four bytes
-      wide and 32-bit aligned.
+      Access width in bytes.  Determines whether the read is performed as
+      an 8-bit, 16-bit, or 32-bit bus access.
+
+  mask:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0xFFFFFFFF
+    description:
+      Mask applied to the value read from the register.  Bits set to 0 in
+      the mask are cleared in the output data.  Default (no mask) passes
+      all bits through.
 
 required:
   - compatible
-  - period
   - reg
+  - period
 
 additionalProperties: false
 
@@ -43,6 +65,8 @@ examples:
   - |
     rng@44 {
         compatible = "timeriomem_rng";
-        reg = <0x44 0x04>;
-        period = <1000000>;
+        reg = <0x44 0x01>;
+        period = <50000>;
+        reg-io-width = <1>;
+        mask = <0xFF>;
     };
-- 
2.54.0

[PATCH v5 2/2] hw_random: timeriomem-rng: add configurable read width and data mask

[Jad Keskes]

The TODO for supporting read sizes other than 32 bits and masking has
been sitting in this driver since 2009.  Implement it.

Add reg-io-width (1, 2, or 4 bytes) and mask support.  The read loop
dispatches on width using readb/readw/readl so a configured 1-byte
access does not trigger a bus error on hardware that rejects 32-bit
reads to that address.  The mask is ANDed with the value before storing.

These are platform properties, not runtime policy -- width depends on
SoC integration, mask reflects which output bits carry entropy.

The alignment check in probe is updated to verify the resource is
aligned to the configured width instead of hardcoding 4-byte alignment.

Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
v5: No changes since v4
v4: Initial version with reg-io-width (bytes) and readb/readw/readl
v3: Added configurable width (bits) with mask
v2: Rebased
v1: Initial submission
---
 drivers/char/hw_random/timeriomem-rng.c | 77 ++++++++++++++++++++-----
 include/linux/timeriomem-rng.h          | 12 ++++
 2 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
index e61f06393209..42393409f22a 100644
--- a/drivers/char/hw_random/timeriomem-rng.c
+++ b/drivers/char/hw_random/timeriomem-rng.c
@@ -14,7 +14,9 @@
  *   has to do is provide the address and 'wait time' that new data becomes
  *   available.
  *
- * TODO: add support for reading sizes other than 32bits and masking
+ * The read width (8, 16, or 32 bits) and an optional data mask can be
+ * configured through platform data or device tree properties.  Default is
+ * 32-bit reads with no mask.
  */
 
 #include <linux/completion.h>
@@ -34,6 +36,8 @@ struct timeriomem_rng_private {
 	void __iomem		*io_base;
 	ktime_t			period;
 	unsigned int		present:1;
+	unsigned int		reg_io_width;
+	u32			mask;
 
 	struct hrtimer		timer;
 	struct completion	completion;
@@ -48,6 +52,7 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
 		container_of(hwrng, struct timeriomem_rng_private, rng_ops);
 	int retval = 0;
 	int period_us = ktime_to_us(priv->period);
+	int chunk = priv->reg_io_width;
 
 	/*
 	 * There may not have been enough time for new data to be generated
@@ -71,11 +76,28 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
 			usleep_range(period_us,
 					period_us + max(1, period_us / 100));
 
-		*(u32 *)data = readl(priv->io_base);
-		retval += sizeof(u32);
-		data += sizeof(u32);
-		max -= sizeof(u32);
-	} while (wait && max > sizeof(u32));
+		switch (priv->reg_io_width) {
+		case 1: {
+			u8 val = readb(priv->io_base) & priv->mask;
+			*(u8 *)data = val;
+			break;
+		}
+		case 2: {
+			u16 val = readw(priv->io_base) & priv->mask;
+			*(u16 *)data = val;
+			break;
+		}
+		case 4: {
+			u32 val = readl(priv->io_base) & priv->mask;
+			*(u32 *)data = val;
+			break;
+		}
+		}
+
+		retval += chunk;
+		data += chunk;
+		max -= chunk;
+	} while (wait && max > chunk);
 
 	/*
 	 * Block any new callers until the RNG has had time to generate new
@@ -125,11 +147,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->io_base))
 		return PTR_ERR(priv->io_base);
 
-	if (res->start % 4 != 0 || resource_size(res) < 4) {
-		dev_err(&pdev->dev,
-			"address must be at least four bytes wide and 32-bit aligned\n");
-		return -EINVAL;
-	}
+	priv->reg_io_width = 4;
+	priv->mask = 0xFFFFFFFF;
 
 	if (pdev->dev.of_node) {
 		int i;
@@ -145,9 +164,41 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
 		if (!of_property_read_u32(pdev->dev.of_node,
 						"quality", &i))
 			priv->rng_ops.quality = i;
+
+		of_property_read_u32(pdev->dev.of_node,
+				     "reg-io-width", &priv->reg_io_width);
+		of_property_read_u32(pdev->dev.of_node,
+				     "mask", &priv->mask);
 	} else {
 		period = pdata->period;
 		priv->rng_ops.quality = pdata->quality;
+
+		if (pdata->reg_io_width_set)
+			priv->reg_io_width = pdata->reg_io_width;
+		if (pdata->mask_set)
+			priv->mask = pdata->mask;
+	}
+
+	if (priv->reg_io_width == 0)
+		priv->reg_io_width = 4;
+
+	switch (priv->reg_io_width) {
+	case 1:
+	case 2:
+	case 4:
+		break;
+	default:
+		dev_err(&pdev->dev, "invalid reg-io-width %u, must be 1, 2, or 4\n",
+			priv->reg_io_width);
+		return -EINVAL;
+	}
+
+	if (!IS_ALIGNED(res->start, priv->reg_io_width) ||
+	    resource_size(res) < priv->reg_io_width) {
+		dev_err(&pdev->dev,
+			"address must be %u-byte aligned\n",
+			priv->reg_io_width);
+		return -EINVAL;
 	}
 
 	priv->period = us_to_ktime(period);
@@ -167,8 +218,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	dev_info(&pdev->dev, "32bits from 0x%p @ %dus\n",
-			priv->io_base, period);
+	dev_info(&pdev->dev, "%u-byte from %p @ %dus\n",
+		 priv->reg_io_width, priv->io_base, period);
 
 	return 0;
 }
diff --git a/include/linux/timeriomem-rng.h b/include/linux/timeriomem-rng.h
index 672df7fbf6c1..5732489a17a1 100644
--- a/include/linux/timeriomem-rng.h
+++ b/include/linux/timeriomem-rng.h
@@ -16,6 +16,18 @@ struct timeriomem_rng_data {
 
 	/* bits of entropy per 1024 bits read */
 	unsigned int		quality;
+
+	/* read width (1, 2, or 4 bytes), 0 means 4 */
+	unsigned int		reg_io_width;
+
+	/* set to true if reg-io-width is explicitly provided */
+	bool			reg_io_width_set;
+
+	/* mask applied to raw read value */
+	u32			mask;
+
+	/* set to true if mask is explicitly provided */
+	bool			mask_set;
 };
 
 #endif /* _LINUX_TIMERIOMEM_RNG_H */
-- 
2.54.0

Re: [PATCH v5 2/2] hw_random: timeriomem-rng: add configurable read width and data mask

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition and broken teardown sequence between module unbind and active hwrng readers causes Deadlocks or Use-After-Free of the private data.
--

commit d12f91b70dc751fd43c31e87b02671ebcad42270
Author: Jad Keskes <inasj268@gmail.com>

hw_random: timeriomem-rng: add configurable read width and data mask

This commit implements configurable read sizes and data masking for the
timeriomem-rng driver. It adds support for 1, 2, or 4-byte reads and an
optional bitmask to filter output entropy, configurable via platform data
or device tree.

> diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
> index e61f063932090..42393409f22ad 100644
> --- a/drivers/char/hw_random/timeriomem-rng.c
> +++ b/drivers/char/hw_random/timeriomem-rng.c

[ ... ]

> @@ -71,11 +76,28 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
>  			usleep_range(period_us,
>  					period_us + max(1, period_us / 100));
>  
> -		*(u32 *)data = readl(priv->io_base);
> -		retval += sizeof(u32);
> -		data += sizeof(u32);
> -		max -= sizeof(u32);
> -	} while (wait && max > sizeof(u32));
> +		switch (priv->reg_io_width) {
> +		case 1: {
> +			u8 val = readb(priv->io_base) & priv->mask;
> +			*(u8 *)data = val;
> +			break;
> +		}
> +		case 2: {
> +			u16 val = readw(priv->io_base) & priv->mask;
> +			*(u16 *)data = val;
> +			break;
> +		}
> +		case 4: {
> +			u32 val = readl(priv->io_base) & priv->mask;
> +			*(u32 *)data = val;
> +			break;
> +		}
> +		}
> +
> +		retval += chunk;
> +		data += chunk;
> +		max -= chunk;
> +	} while (wait && max > chunk);
>  
>  	/*
>  	 * Block any new callers until the RNG has had time to generate new

[Severity: High]
This is a pre-existing issue, but can this sequence lead to deadlocks or a 
use-after-free during module teardown?

Because the driver uses devm_hwrng_register(), devres unregisters the RNG
after the driver's remove function returns. The timeriomem_rng_remove function
explicitly calls hrtimer_cancel(&priv->timer), but concurrent readers might
still be active in timeriomem_rng_read.

If a reader is waiting earlier in this function at
wait_for_completion(&priv->completion) when the timer is cancelled, won't it
hang forever since the timer will never fire to wake it up?

Alternatively, if the reader is already past the wait, it will unconditionally
call hrtimer_restart(&priv->timer) slightly further down at the end of this
function. Doesn't this re-arm the timer after it was cancelled by the removal
thread?

If the timer is re-armed and devres subsequently frees the
timeriomem_rng_private data, won't the timer eventually fire and access freed
memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618120110.36439-1-inasj268@gmail.com?part=2