Re: [PATCH v4 1/3] hwspinlock: Convert to XArray

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

> [Severity: High]
> This is a pre-existing issue, but does dereferencing hwlock->bank->dev under
> rcu_read_lock() and later accessing hwlock->bank->num_locks after dropping it
> in of_hwspin_lock_get_id() lead to a Use-After-Free?
> 
> While the XArray (and previously Radix Tree) lookup yields a pointer to
> hwlock, the hwlock structure is managed by devres (e.g. devm_kzalloc) which
> frees the memory synchronously on device unbind without waiting for an RCU
> grace period.

My next hwspinlock series will tackle the plenty of UAF issues.

> [Severity: High]
> This is a pre-existing issue, but does aborting here lead to dangling
> pointers in the XArray when devres subsequently frees the bank memory?

Ditto.

> > +	xas_unlock(&xas);
> >  
> > +	ret = hwspin_lock_prepare(hwlock);
> > +
> > +	xas_lock(&xas);
> > +	if (ret < 0) {
> > +		hwlock = NULL;
> > +		xas_set_mark(&xas, HWSPINLOCK_UNUSED);
> 
> [Severity: High]
> Can reusing the un-reset XA_STATE after dropping the XArray lock and sleeping
> in hwspin_lock_request_specific() lead to a Use-After-Free?
> 
> This function drops the XArray lock and calls hwspin_lock_prepare(), which can
> sleep via pm_runtime_get_sync(). While sleeping, concurrent modifications to
> the XArray can cause internal nodes to be reallocated or freed.
> 
> Upon waking, if hwspin_lock_prepare() fails, the function reacquires the lock
> and calls xas_set_mark(&xas, HWSPINLOCK_UNUSED) using the stale xas iterator,
> which might dereference the potentially freed xas.xa_node.

I fixed this in v5 to be sent soon.