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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free / Deadlock in the device removal path due to incorrect devres ordering.
--

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

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

This commit implements configurable read widths (8, 16, or 32 bits) and data
masking for the timeriomem-rng driver. It updates the read loop to dispatch
based on width to avoid bus errors, and uses properties from 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..4557326618c90 100644
> --- a/drivers/char/hw_random/timeriomem-rng.c
> +++ b/drivers/char/hw_random/timeriomem-rng.c
[ ... ]
> @@ -167,8 +219,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, "%ubit from %p @ %dus\n",
> +		 priv->width, priv->io_base, period);
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but looking at the probe function
above, it uses devm_hwrng_register() to register the RNG device. This means
that hwrng_unregister() is automatically called by devres after
timeriomem_rng_remove() returns.

In timeriomem_rng_remove(), the driver manually calls
hrtimer_cancel(&priv->timer). At this point, the RNG is still registered and
fully exposed to concurrent readers. This ordering creates two distinct issues:

1. Deadlock: If a concurrent reader is inside timeriomem_rng_read() and
blocking on wait_for_completion(&priv->completion), it will wait forever
because the timer was just cancelled. When devres subsequently calls
hwrng_unregister(), it waits indefinitely for the reader's reference count to
drop, deadlocking the unbind process.

2. Use-After-Free: If a concurrent reader finishes its read loop just after
remove() cancelled the timer, it unconditionally executes
hrtimer_restart(&priv->timer) in the read path:

timeriomem_rng_read() {
    ...
    priv->present = 0;
    reinit_completion(&priv->completion);
    hrtimer_forward_now(&priv->timer, priv->period);
    hrtimer_restart(&priv->timer);
    ...
}

The reader then completes, remove() finishes, and devres frees the priv
structure. The re-armed timer will then fire and dereference the freed priv
pointer.

Could this devres ordering issue be addressed by manually unregistering the
hwrng before cancelling the timer in remove(), or by using a custom devm
action to ensure correct teardown order?

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