Do not modify imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp, until the format has been correctly validated.

Otherwise, even if we use a backup variable, there is a period of time where imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp might have an invalid value that can be used by other functions.

Cc: stable@vger.kernel.org Fixes: ad91849996f9 ("media: staging/intel-ipu3: Fix set_fmt error handling") Reviewed-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Ricardo Ribalda ribalda@chromium.org Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com --- drivers/staging/media/ipu3/ipu3-v4l2.c | 30 ++++++++++++-------------- 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c index 9e8980b34547..6d9c49b39531 100644 --- a/drivers/staging/media/ipu3/ipu3-v4l2.c +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c @@ -669,7 +669,6 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node, struct imgu_css_pipe *css_pipe = &imgu->css.pipes[pipe]; struct imgu_media_pipe *imgu_pipe = &imgu->imgu_pipe[pipe]; struct imgu_v4l2_subdev *imgu_sd = &imgu_pipe->imgu_sd; - struct v4l2_pix_format_mplane fmt_backup;

dev_dbg(dev, "set fmt node [%u][%u](try = %u)", pipe, node, try);

@@ -687,6 +686,7 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node,

dev_dbg(dev, "IPU3 pipe %u pipe_id = %u", pipe, css_pipe->pipe_id);

+ css_q = imgu_node_to_queue(node); for (i = 0; i < IPU3_CSS_QUEUES; i++) { unsigned int inode = imgu_map_node(imgu, i);

@@ -701,6 +701,11 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node, continue; }

+ if (i == css_q) { + fmts[i] = &f->fmt.pix_mp; + continue; + } + if (try) { fmts[i] = kmemdup(&imgu_pipe->nodes[inode].vdev_fmt.fmt.pix_mp, sizeof(struct v4l2_pix_format_mplane), @@ -729,39 +734,32 @@ static int imgu_fmt(struct imgu_device *imgu, unsigned int pipe, int node, rects[IPU3_CSS_RECT_GDC]->height = pad_fmt.height; }

- /* - * imgu doesn't set the node to the value given by user - * before we return success from this function, so set it here. - */ - css_q = imgu_node_to_queue(node); if (!fmts[css_q]) { ret = -EINVAL; goto out; } - fmt_backup = *fmts[css_q]; - *fmts[css_q] = f->fmt.pix_mp;

if (try) ret = imgu_css_fmt_try(&imgu->css, fmts, rects, pipe); else ret = imgu_css_fmt_set(&imgu->css, fmts, rects, pipe);

- if (try || ret < 0) - *fmts[css_q] = fmt_backup; - /* ret is the binary number in the firmware blob */ if (ret < 0) goto out;

- if (try) - f->fmt.pix_mp = *fmts[css_q]; - else - f->fmt = imgu_pipe->nodes[node].vdev_fmt.fmt; + /* + * imgu doesn't set the node to the value given by user + * before we return success from this function, so set it here. + */ + if (!try) + imgu_pipe->nodes[node].vdev_fmt.fmt.pix_mp = f->fmt.pix_mp;

out: if (try) { for (i = 0; i < IPU3_CSS_QUEUES; i++) - kfree(fmts[i]); + if (i != css_q) + kfree(fmts[i]); }

return ret;