On 08/09/2022 01:44, Maximilian Luz wrote:
Calling v4l2_subdev_get_try_crop() and v4l2_subdev_get_try_compose() with a subdev state of NULL leads to a NULL pointer dereference. This can currently happen in imgu_subdev_set_selection() when the state passed in is NULL, as this method first gets pointers to both the "try" and "active" states and only then decides which to use. The same issue has been addressed for imgu_subdev_get_selection() with commit 30d03a0de650 ("ipu3-imgu: Fix NULL pointer dereference in active selection access"). However the issue still persists in imgu_subdev_set_selection(). Therefore, apply a similar fix as done in the aforementioned commit to imgu_subdev_set_selection(). To keep things a bit cleaner, introduce helper functions for "crop" and "compose" access and use them in both imgu_subdev_set_selection() and imgu_subdev_get_selection(). Fixes: 0d346d2a6f54 ("media: v4l2-subdev: add subdev-wide state struct") Cc: stable@xxxxxxxxxxxxxxx # for v5.14 and later Signed-off-by: Maximilian Luz <luzmaximilian@xxxxxxxxx> --- drivers/staging/media/ipu3/ipu3-v4l2.c | 57 +++++++++++++++----------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c index ce13e746c15f..e530767e80a5 100644 --- a/drivers/staging/media/ipu3/ipu3-v4l2.c +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c @@ -188,6 +188,28 @@ static int imgu_subdev_set_fmt(struct v4l2_subdev *sd, return 0; }+static struct v4l2_rect *+imgu_subdev_get_crop(struct imgu_v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, unsigned int pad, + enum v4l2_subdev_format_whence which) +{ + if (which == V4L2_SUBDEV_FORMAT_TRY) + return v4l2_subdev_get_try_crop(&sd->subdev, sd_state, pad); + else + return &sd->rect.eff; +} + +static struct v4l2_rect * +imgu_subdev_get_compose(struct imgu_v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, unsigned int pad, + enum v4l2_subdev_format_whence which) +{ + if (which == V4L2_SUBDEV_FORMAT_TRY) + return v4l2_subdev_get_try_compose(&sd->subdev, sd_state, pad); + else + return &sd->rect.bds; +}
If I understand right, these functions are only called with pad 0 (IMGU_NODE_IN). I would drop the pad argument here and use IMGU_NODE_IN. Otherwise it gives a false idea that other pads could be used with these functions, and that would fail for the V4L2_SUBDEV_FORMAT_ACTIVE case.
However, that's not a big issue. With or without the change: Reviewed-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> Tomi
