[2/2] media: stm32: dcmi: Fix subdev op call with uninitialized state
Commit Message
stm32-dcmi calls its source subdev with v4l2_subdev_call() using a
v4l2_subdev_state constructed on stack. This means that init_cfg is
never called for that state, and a source subdev that depends on the
init_cfg call may break.
A new macro has been added for this particular purpose, which properly
initializes the state, so let's use v4l2_subdev_call_state_try() here.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
drivers/media/platform/st/stm32/stm32-dcmi.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
Comments
Hi Tomi,
Thank you for the patch.
On Fri, Jul 01, 2022 at 04:15:59PM +0300, Tomi Valkeinen wrote:
> stm32-dcmi calls its source subdev with v4l2_subdev_call() using a
> v4l2_subdev_state constructed on stack. This means that init_cfg is
> never called for that state, and a source subdev that depends on the
> init_cfg call may break.
>
> A new macro has been added for this particular purpose, which properly
> initializes the state, so let's use v4l2_subdev_call_state_try() here.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/media/platform/st/stm32/stm32-dcmi.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c
> index 09a743cd7004..eb831b5932e7 100644
> --- a/drivers/media/platform/st/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c
> @@ -999,10 +999,6 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
> const struct dcmi_format *sd_fmt;
> struct dcmi_framesize sd_fsize;
> struct v4l2_pix_format *pix = &f->fmt.pix;
> - struct v4l2_subdev_pad_config pad_cfg;
> - struct v4l2_subdev_state pad_state = {
> - .pads = &pad_cfg
> - };
> struct v4l2_subdev_format format = {
> .which = V4L2_SUBDEV_FORMAT_TRY,
> };
> @@ -1037,8 +1033,7 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
> }
>
> v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
> - ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
> - &pad_state, &format);
> + ret = v4l2_subdev_call_state_try(dcmi->source, pad, set_fmt, &format);
> if (ret < 0)
> return ret;
>
> @@ -1187,10 +1182,6 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
> struct v4l2_subdev_format format = {
> .which = V4L2_SUBDEV_FORMAT_TRY,
> };
> - struct v4l2_subdev_pad_config pad_cfg;
> - struct v4l2_subdev_state pad_state = {
> - .pads = &pad_cfg
> - };
> int ret;
>
> sd_fmt = find_format_by_fourcc(dcmi, pix->pixelformat);
> @@ -1203,8 +1194,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
> }
>
> v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
> - ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
> - &pad_state, &format);
> + ret = v4l2_subdev_call_state_try(dcmi->source, pad, set_fmt, &format);
> if (ret < 0)
> return ret;
>
On 7/1/22 15:15, Tomi Valkeinen wrote:
> stm32-dcmi calls its source subdev with v4l2_subdev_call() using a
> v4l2_subdev_state constructed on stack. This means that init_cfg is
> never called for that state, and a source subdev that depends on the
> init_cfg call may break.
>
> A new macro has been added for this particular purpose, which properly
> initializes the state, so let's use v4l2_subdev_call_state_try() here.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Thanks!
Hans
> ---
> drivers/media/platform/st/stm32/stm32-dcmi.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c
> index 09a743cd7004..eb831b5932e7 100644
> --- a/drivers/media/platform/st/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c
> @@ -999,10 +999,6 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
> const struct dcmi_format *sd_fmt;
> struct dcmi_framesize sd_fsize;
> struct v4l2_pix_format *pix = &f->fmt.pix;
> - struct v4l2_subdev_pad_config pad_cfg;
> - struct v4l2_subdev_state pad_state = {
> - .pads = &pad_cfg
> - };
> struct v4l2_subdev_format format = {
> .which = V4L2_SUBDEV_FORMAT_TRY,
> };
> @@ -1037,8 +1033,7 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
> }
>
> v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
> - ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
> - &pad_state, &format);
> + ret = v4l2_subdev_call_state_try(dcmi->source, pad, set_fmt, &format);
> if (ret < 0)
> return ret;
>
> @@ -1187,10 +1182,6 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
> struct v4l2_subdev_format format = {
> .which = V4L2_SUBDEV_FORMAT_TRY,
> };
> - struct v4l2_subdev_pad_config pad_cfg;
> - struct v4l2_subdev_state pad_state = {
> - .pads = &pad_cfg
> - };
> int ret;
>
> sd_fmt = find_format_by_fourcc(dcmi, pix->pixelformat);
> @@ -1203,8 +1194,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
> }
>
> v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
> - ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
> - &pad_state, &format);
> + ret = v4l2_subdev_call_state_try(dcmi->source, pad, set_fmt, &format);
> if (ret < 0)
> return ret;
>
On 7/1/22 15:15, Tomi Valkeinen wrote:
> stm32-dcmi calls its source subdev with v4l2_subdev_call() using a
> v4l2_subdev_state constructed on stack. This means that init_cfg is
> never called for that state, and a source subdev that depends on the
> init_cfg call may break.
>
> A new macro has been added for this particular purpose, which properly
> initializes the state, so let's use v4l2_subdev_call_state_try() here.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tested-by: Marek Vasut <marex@denx.de>
@@ -999,10 +999,6 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
const struct dcmi_format *sd_fmt;
struct dcmi_framesize sd_fsize;
struct v4l2_pix_format *pix = &f->fmt.pix;
- struct v4l2_subdev_pad_config pad_cfg;
- struct v4l2_subdev_state pad_state = {
- .pads = &pad_cfg
- };
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
@@ -1037,8 +1033,7 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
}
v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
- ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
- &pad_state, &format);
+ ret = v4l2_subdev_call_state_try(dcmi->source, pad, set_fmt, &format);
if (ret < 0)
return ret;
@@ -1187,10 +1182,6 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
- struct v4l2_subdev_pad_config pad_cfg;
- struct v4l2_subdev_state pad_state = {
- .pads = &pad_cfg
- };
int ret;
sd_fmt = find_format_by_fourcc(dcmi, pix->pixelformat);
@@ -1203,8 +1194,7 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
}
v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
- ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
- &pad_state, &format);
+ ret = v4l2_subdev_call_state_try(dcmi->source, pad, set_fmt, &format);
if (ret < 0)
return ret;