Commit ecefa105 authored by Laurent Pinchart's avatar Laurent Pinchart Committed by Hans Verkuil
Browse files

media: Zero-initialize all structures passed to subdev pad operations 



Several drivers call subdev pad operations, passing structures that are
not fully zeroed. While the drivers initialize the fields they care
about explicitly, this results in reserved fields having uninitialized
values. Future kernel API changes that make use of those fields thus
risk breaking proper driver operation in ways that could be hard to
detect.

To avoid this, make the code more robust by zero-initializing all the
structures passed to subdev pad operation. Maintain a consistent coding
style by preferring designated initializers (which zero-initialize all
the fields that are not specified) over memset() where possible, and
make variable declarations local to inner scopes where applicable. One
notable exception to this rule is in the ipu3 driver, where a memset()
is needed as the structure is not a local variable but a function
parameter provided by the caller.

Not all fields of those structures can be initialized when declaring the
variables, as the values for those fields are computed later in the
code. Initialize the 'which' field in all cases, and other fields when
the variable declaration is so close to the v4l2_subdev_call() call that
it keeps all the context easily visible when reading the code, to avoid
hindering readability.

Signed-off-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Shuah Khan <skhan@linuxfoundation.org> # For vimc
Reviewed-by: Lad Prabhakar <prabhakar.csengg@gmail.com> # For am437x
Acked-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: default avatarTomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: default avatarKieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> # For drivers/staging/media/imx/
Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
parent a8ca0cf1

drivers/media/pci/cobalt/cobalt-v4l2.c

+10 −6
Original line number Diff line number Diff line
@@ -708,7 +708,6 @@ static int cobalt_g_fmt_vid_cap(struct file *file, void *priv_fh, 
{

	struct cobalt_stream *s = video_drvdata(file);

	struct v4l2_pix_format *pix = &f->fmt.pix;

	struct v4l2_subdev_format sd_fmt;



	pix->width = s->width;

	pix->height = s->height;
@@ -718,8 +717,11 @@ static int cobalt_g_fmt_vid_cap(struct file *file, void *priv_fh, 
	if (s->input == 1) {

		pix->colorspace = V4L2_COLORSPACE_SRGB;

	} else {

		sd_fmt.pad = s->pad_source;

		sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;

		struct v4l2_subdev_format sd_fmt = {

			.pad = s->pad_source,

			.which = V4L2_SUBDEV_FORMAT_ACTIVE,

		};



		v4l2_subdev_call(s->sd, pad, get_fmt, NULL, &sd_fmt);

		v4l2_fill_pix_format(pix, &sd_fmt.format);

	}
@@ -735,7 +737,6 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv_fh, 
{

	struct cobalt_stream *s = video_drvdata(file);

	struct v4l2_pix_format *pix = &f->fmt.pix;

	struct v4l2_subdev_format sd_fmt;



	/* Check for min (QCIF) and max (Full HD) size */

	if ((pix->width < 176) || (pix->height < 144)) {
@@ -760,8 +761,11 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv_fh, 
		pix->height = 1080;

		pix->colorspace = V4L2_COLORSPACE_SRGB;

	} else {

		sd_fmt.pad = s->pad_source;

		sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;

		struct v4l2_subdev_format sd_fmt = {

			.pad = s->pad_source,

			.which = V4L2_SUBDEV_FORMAT_ACTIVE,

		};



		v4l2_subdev_call(s->sd, pad, get_fmt, NULL, &sd_fmt);

		v4l2_fill_pix_format(pix, &sd_fmt.format);

	}

drivers/media/pci/intel/ipu3/ipu3-cio2-main.c

+1 −0
Original line number Diff line number Diff line
@@ -1305,6 +1305,7 @@ static int cio2_subdev_link_validate_get_format(struct media_pad *pad, 
		struct v4l2_subdev *sd =

			media_entity_to_v4l2_subdev(pad->entity);



		memset(fmt, 0, sizeof(*fmt));

		fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;

		fmt->pad = pad->index;

		return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);

drivers/media/platform/qcom/camss/camss-video.c

+3 −2
Original line number Diff line number Diff line
@@ -342,7 +342,9 @@ static struct v4l2_subdev *video_remote_subdev(struct camss_video *video, 
static int video_get_subdev_format(struct camss_video *video,

				   struct v4l2_format *format)

{

	struct v4l2_subdev_format fmt;

	struct v4l2_subdev_format fmt = {

		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

	};

	struct v4l2_subdev *subdev;

	u32 pad;

	int ret;
@@ -353,7 +355,6 @@ static int video_get_subdev_format(struct camss_video *video, 


	memset(&fmt, 0, sizeof(fmt));

	fmt.pad = pad;

	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;



	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);

	if (ret)

drivers/media/platform/renesas/vsp1/vsp1_video.c

+3 −2
Original line number Diff line number Diff line
@@ -62,7 +62,9 @@ vsp1_video_remote_subdev(struct media_pad *local, u32 *pad) 


static int vsp1_video_verify_format(struct vsp1_video *video)

{

	struct v4l2_subdev_format fmt;

	struct v4l2_subdev_format fmt = {

		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

	};

	struct v4l2_subdev *subdev;

	int ret;
@@ -70,7 +72,6 @@ static int vsp1_video_verify_format(struct vsp1_video *video) 
	if (subdev == NULL)

		return -EINVAL;



	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;

	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);

	if (ret < 0)

		return ret == -ENOIOCTLCMD ? -EINVAL : ret;

drivers/media/platform/samsung/exynos4-is/fimc-capture.c

+7 −4
Original line number Diff line number Diff line
@@ -854,7 +854,7 @@ static int fimc_get_sensor_frame_desc(struct v4l2_subdev *sensor, 
				      struct v4l2_plane_pix_format *plane_fmt,

				      unsigned int num_planes, bool try)

{

	struct v4l2_mbus_frame_desc fd;

	struct v4l2_mbus_frame_desc fd = { };

	int i, ret;

	int pad;
@@ -1095,7 +1095,12 @@ static int fimc_cap_g_input(struct file *file, void *priv, unsigned int *i) 
 */

static int fimc_pipeline_validate(struct fimc_dev *fimc)

{

	struct v4l2_subdev_format sink_fmt, src_fmt;

	struct v4l2_subdev_format sink_fmt = {

		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

	};

	struct v4l2_subdev_format src_fmt = {

		.which = V4L2_SUBDEV_FORMAT_ACTIVE,

	};

	struct fimc_vid_cap *vc = &fimc->vid_cap;

	struct v4l2_subdev *sd = &vc->subdev;

	struct fimc_pipeline *p = to_fimc_pipeline(vc->ve.pipe);
@@ -1132,7 +1137,6 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc) 
			sink_fmt.format.code = ff->fmt ? ff->fmt->mbus_code : 0;

		} else {

			sink_fmt.pad = sink_pad->index;

			sink_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;

			ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sink_fmt);

			if (ret < 0 && ret != -ENOIOCTLCMD)

				return -EPIPE;
@@ -1141,7 +1145,6 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc) 
		/* Retrieve format at the source pad */

		sd = media_entity_to_v4l2_subdev(src_pad->entity);

		src_fmt.pad = src_pad->index;

		src_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;

		ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &src_fmt);

		if (ret < 0 && ret != -ENOIOCTLCMD)

			return -EPIPE;