Hi, On Thu, 25 Aug 2022 22:03:51 +0300, Sakari Ailus wrote:
What the IMGU driver did was that it first acquired the pointers to active and try V4L2 subdev state, and only then figured out which one to use. The problem with that approach and a later patch (see Fixes: tag) is that as sd_state argument to v4l2_subdev_get_try_crop() et al is NULL, there is now an attempt to dereference that. Fix this.
Thank you for this fix, this however only addresses imgu_subdev_get_selection(), but the issue is also present in imgu_subdev_set_selection(). I assume that a similar fix is needed for that. I've added a diff for that below. Feel free to squash that into your patch or let me know if I should submit this separately. Regards, Max --- drivers/staging/media/ipu3/ipu3-v4l2.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c index 2234bb8d48b3..079f2635c70d 100644 --- a/drivers/staging/media/ipu3/ipu3-v4l2.c +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c @@ -223,10 +223,9 @@ static int imgu_subdev_set_selection(struct v4l2_subdev *sd, struct v4l2_subdev_selection *sel) { struct imgu_device *imgu = v4l2_get_subdevdata(sd); - struct imgu_v4l2_subdev *imgu_sd = container_of(sd, - struct imgu_v4l2_subdev, - subdev); - struct v4l2_rect *rect, *try_sel; + struct imgu_v4l2_subdev *imgu_sd = + container_of(sd, struct imgu_v4l2_subdev, subdev); + struct v4l2_rect *rect; dev_dbg(&imgu->pci_dev->dev, "set subdev %u sel which %u target 0x%4x rect [%ux%u]", @@ -238,22 +237,22 @@ static int imgu_subdev_set_selection(struct v4l2_subdev *sd, switch (sel->target) { case V4L2_SEL_TGT_CROP: - try_sel = v4l2_subdev_get_try_crop(sd, sd_state, sel->pad); - rect = &imgu_sd->rect.eff; + if (sel->which == V4L2_SUBDEV_FORMAT_TRY) + rect = v4l2_subdev_get_try_crop(sd, sd_state, sel->pad); + else + rect = &imgu_sd->rect.eff; break; case V4L2_SEL_TGT_COMPOSE: - try_sel = v4l2_subdev_get_try_compose(sd, sd_state, sel->pad); - rect = &imgu_sd->rect.bds; + if (sel->which == V4L2_SUBDEV_FORMAT_TRY) + rect = v4l2_subdev_get_try_compose(sd, sd_state, sel->pad); + else + rect = &imgu_sd->rect.bds; break; default: return -EINVAL; } - if (sel->which == V4L2_SUBDEV_FORMAT_TRY) - *try_sel = sel->r; - else - *rect = sel->r; - + *rect = sel->r; return 0; } -- 2.37.3