Re: [PATCH] media: i2c: imx471: Fix uninitialized error value in imx471_set_ctrl()

[Kate Hsuan]

Hi David,

Thank you for the fix.

On Fri, Jul 24, 2026 at 12:14 PM David Carlier <devnexen@xxxxxxxxx> wrote:
>
> The exposure control write passes the address of the local ret variable
> to cci_write() as its error pointer while ret is still uninitialized.
> cci_write() returns early without touching the hardware when the error
> value it is handed is already non-zero, so a non-zero garbage value on
> the stack silently skips programming the exposure register and makes the
> control write return a bogus, possibly positive, status.
>
> This path runs on every stream start and on every exposure update from
> userspace. Initialize ret to zero so the exposure register is always
> written and a correct status is returned.
>
> Fixes: be1589e567ae ("media: i2c: imx471: Add Sony IMX471 image sensor driver")
> Signed-off-by: David Carlier <devnexen@xxxxxxxxx>
> ---
>  drivers/media/i2c/imx471.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> index 6d358b11e96d..5aca5f2355d6 100644
> --- a/drivers/media/i2c/imx471.c
> +++ b/drivers/media/i2c/imx471.c
> @@ -306,7 +306,7 @@ static int imx471_set_ctrl(struct v4l2_ctrl *ctrl)
>                         v4l2_subdev_get_locked_active_state(&sensor->sd);
>         const struct v4l2_mbus_framefmt *format =
>                         v4l2_subdev_state_get_format(state, 0);
> -       int ret;
> +       int ret = 0;

Setting ret to 0 can resolve the issue, but the root cause is at lines
337 and 342. Since we don't need the previous error state when calling
cci_write() in imx471_set_ctrl(), the fourth parameter of cci_write()
should be NULL.
So the fix for the V4L2_CID_EXPOSURE and V4L2_CID_VBLANK is

case V4L2_CID_EXPOSURE:
    ret = cci_write(sensor->regmap, IMX471_REG_EXPOSURE, ctrl->val, NULL);
    break;
case V4L2_CID_VBLANK:
    /* Update FLL that meets expected vertical blanking */
    ret = cci_write(sensor->regmap, IMX471_REG_FLL, format->height +
ctrl->val, NULL);
    break;

This ensures the cci_write() writes the data to the register without
being influenced by an uninitialized ret value and the ret can be set
properly.

>
>         if (ctrl->id == V4L2_CID_VBLANK) {
>                 s64 exposure_max = format->height + ctrl->val -
> --
> 2.53.0
>


-- 
BR,
Kate