Re: [PATCH] i2c: imx-lpi2c: avoid accessing target registers on master-only controllers

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

Pre-existing issues:
- [High] Missing rollback in `lpi2c_imx_register_target()` when `pm_runtime=
_resume_and_get()` fails, leaving a dangling pointer in `lpi2c_imx->target`.
--

--- Patch [1]: [PATCH] i2c: imx-lpi2c: avoid accessing target registers on =
master-only controllers ---
commit c2a5eb969b33b95968f240043a2bfa7ba6e85caf
Author: Carlos Song <[email protected]>

i2c: imx-lpi2c: avoid accessing target registers on master-only controllers

This commit reads the VERID register during probe to check for target mode
support. It prevents accessing target registers on master-only controllers
and rejects target registration requests if unsupported.

> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-=
imx-lpi2c.c
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
[ ... ]
> @@ -1330,6 +1335,10 @@ static int lpi2c_imx_register_target(struct i2c_cl=
ient *client)
>  	struct lpi2c_imx_struct *lpi2c_imx =3D i2c_get_adapdata(client->adapter=
);
>  	int ret;
> =20
> +	/* Reject target-mode registration on controllers that don't support it=
. */
> +	if (!lpi2c_imx->target_supported)
> +		return -EOPNOTSUPP;
> +
>  	if (lpi2c_imx->target)
>  		return -EBUSY;
> =20
>  	lpi2c_imx->target =3D client;
> =20
>  	ret =3D pm_runtime_resume_and_get(lpi2c_imx->adapter.dev.parent);
>  	if (ret < 0) {
>  		dev_err(&lpi2c_imx->adapter.dev, "failed to resume i2c controller");
>  		return ret;
>  	}

[Severity: High]
This is a pre-existing issue, but does this error path leave a dangling
pointer in the controller data?

If pm_runtime_resume_and_get() fails in lpi2c_imx_register_target(), the
function returns without clearing lpi2c_imx->target. Because the registrati=
on
failed, the target driver probe will fail and the i2c_client memory can be
freed (e.g., if dynamically removed or unbound).

Later, if the system is suspended and resumed, lpi2c_resume_noirq() will see
that lpi2c_imx->target is non-NULL and unconditionally dereference it:

drivers/i2c/busses/i2c-imx-lpi2c.c:lpi2c_resume_noirq() {
    ...
    if (lpi2c_imx->target)
        lpi2c_imx_target_init(lpi2c_imx);
    ...
}

Should lpi2c_imx->target be set back to NULL in this error path to avoid a
potential use-after-free crash?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803032705.2250=
[email protected]?part=3D1