Re: [PATCH v13 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double-unlock and missing error propagation in `p3h2x4x_reg` lock guard on `regmap_read()` failure.
--

commit ec428ff479eee2740ff39044d74b67527f18c3e6
Author: Aman Kumar Pandey <[email protected]>

regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub

The NXP P3H2x4x family integrates on-die regulators alongside I3C hub
functionality. This driver registers the regulators using the MFD
framework and exposes them via the regulator subsystem.

> diff --git a/drivers/regulator/p3h2840_i3c_hub_regulator.c b/drivers/regulator/p3h2840_i3c_hub_regulator.c
> new file mode 100644
> index 0000000000000..781622a6ff467
> --- /dev/null
> +++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
[ ... ]
> +static void p3h2x4x_reg_guard_enter(struct regulator_dev *rdev,
> +				    struct p3h2x4x_reg_state *state)
> +{
> +	struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
> +
> +	mutex_lock(&priv->lock);
> +	state->restore = false;
> +
> +	if (regmap_read(rdev->regmap,
> +			P3H2X4X_DEV_REG_PROTECTION_CODE,
> +			&state->orig)) {
> +		mutex_unlock(&priv->lock);
> +		return;
> +	}

[Severity: High]
If regmap_read() fails here, we unlock the mutex and return. 

Since this function is used as the lock guard constructor, the caller is not
aware of the failure. Does this allow the caller to proceed and execute the
guarded critical section without holding the lock?

For example, in p3h2x4x_regulator_enable():

	guard(p3h2x4x_reg)(rdev);
	return regulator_enable_regmap(rdev);

Would regulator_enable_regmap() execute concurrently without the lock if the
initialization read fails?

[ ... ]
> +static void p3h2x4x_reg_guard_exit(struct regulator_dev *rdev,
> +				   struct p3h2x4x_reg_state *state)
> +{
> +	struct p3h2x4x_regulator_dev *priv = rdev_get_drvdata(rdev);
> +
> +	if (state->restore)
> +		regmap_write(rdev->regmap,
> +			     P3H2X4X_DEV_REG_PROTECTION_CODE,
> +			     state->orig);
> +
> +	mutex_unlock(&priv->lock);
> +}

[Severity: High]
When the lock guard goes out of scope, p3h2x4x_reg_guard_exit() is invoked
unconditionally. 

If we previously hit the regmap_read() error path in the constructor and
already unlocked the mutex, will this second call to mutex_unlock(&priv->lock)
cause a double-unlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c