Re: [PATCH v4 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] Incorrect teardown order between manual `hrtimer_cancel()` in `remove()` and devres-managed hwrng unregistration leads to deadlocks or Use-After-Free vulnerabilities.
--

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

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

This commit implements configurable read sizes (1, 2, or 4 bytes) and data
masking for the timeriomem-rng driver. It updates the read loop to dispatch
on the configured width and updates the alignment checks in the probe function.

> diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
> --- 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) {
[ ... ]
> +		}
> +
> +		retval += chunk;
> +		data += chunk;
> +		max -= chunk;
> +	} while (wait && max > chunk);

[Severity: High]
This isn't a bug introduced by this patch, but looking at the driver's
teardown sequence, is there a race condition between devres unregistration
and the hrtimer?

The driver registers the RNG via devm_hwrng_register(), meaning devres handles
unregistration after timeriomem_rng_remove() returns. In
timeriomem_rng_remove(), the driver manually calls hrtimer_cancel().

Since the device remains exposed to userspace during this window, can this
lead to a deadlock? If a reader is waiting in timeriomem_rng_read() on
wait_for_completion(), cancelling the timer ensures it never wakes up. The
reader holds a reference to the hwrng, causing devres unregistration to block
indefinitely in hwrng_unregister().

Additionally, can this lead to a use-after-free? If a read occurs after
hrtimer_cancel() completes but before devres unregisters the RNG,
timeriomem_rng_read() will unconditionally execute hrtimer_restart():

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

    return retval;
}

If devres then frees the timeriomem_rng_private structure, wouldn't the timer
fire and cause timeriomem_rng_trigger() to dereference freed memory?

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