Re: [PATCH 2/2] media: i2c: ak7375: Do not mark the VCM active before powering it

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The commit message incorrectly attributes a regulator leak to `pm_runtime_idle()`, which is structurally impossible.

Pre-existing issues:
- [Critical] Use-After-Free of `struct ak7375_device` on driver unbind due to `devm_kzalloc`.
--

commit a4a5fe00ce60666a1ebfccbdf63f44f93d436c10
Author: Jan-Michael Brummer <jan.brummer@tabos.org>

media: i2c: ak7375: Do not mark the VCM active before powering it

This patch replaces pm_runtime_set_active() with pm_runtime_set_suspended()
in the probe function to correctly reflect that the regulators are initially
off. It also adds a call to disable the regulators in the resume error path
to prevent leaking an enable count.

> probe() calls pm_runtime_set_active() while the regulators are still
> off, so runtime PM believes a powered device is present.  The following
> pm_runtime_idle() runs the suspend path against an unpowered chip, which
> fails its I2C writes and then calls regulator_bulk_disable() on
> regulators that were never enabled - later tripping the enable/disable
> balance check in regulator_bulk_free() on unbind.

[Severity: Medium]
Is this description accurate?

Looking at ak7375_vcm_suspend(), it explicitly checks the active state and
returns early:

drivers/media/i2c/ak7375.c:ak7375_vcm_suspend() {
    ...
    if (!ak7375_dev->active)
        return 0;
    ...
}

Since ak7375_dev is zero-initialized by devm_kzalloc() during probe, active
is false. This means pm_runtime_idle() would never reach the I2C writes or
regulator_bulk_disable() during probe.

Could the actual leak have occurred strictly on the resume error path instead?

> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> index 4f311c4c06ba0..2763bbed6cbd4 100644
> --- a/drivers/media/i2c/ak7375.c
> +++ b/drivers/media/i2c/ak7375.c
> @@ -235,9 +235,12 @@ static int ak7375_probe(struct i2c_client *client)
>  	if (ret < 0)
>  		goto err_cleanup;
>  
> -	pm_runtime_set_active(&client->dev);
> +	/*
> +	 * The regulators are off at this point, so the device is suspended,
> +	 * not active.
> +	 */
> +	pm_runtime_set_suspended(&client->dev);
>  	pm_runtime_enable(&client->dev);
> -	pm_runtime_idle(&client->dev);
>  
>  	return 0;

[Severity: High]
This is a pre-existing issue, but does this devres cleanup lead to a
use-after-free on driver unbind?

The ak7375_device structure is allocated with devm_kzalloc() earlier in
ak7375_probe(). If a user-space process holds the /dev/v4l-subdevX node open
and the driver is unbound (e.g., via sysfs), ak7375_remove() will complete
and trigger devres to free ak7375_device.

When the process later closes the file descriptor, the V4L2 core functions
(such as subdev_close calling sd->internal_ops->close) will dereference the
now-freed v4l2_subdev pointer.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829052514.18178-1-jan.brummer@tabos.org?part=2