Commit 34e77ed8 authored by Laurent Pinchart's avatar Laurent Pinchart Committed by Mauro Carvalho Chehab
Browse files

[media] v4l: vsp1: Protect against race conditions between get and set format



The subdev userspace API isn't serialized in the core, serialize access
to formats and selection rectangles in the driver.

Signed-off-by: default avatarLaurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: default avatarNiklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@s-opensource.com>
parent e4e70a14
Loading
Loading
Loading
Loading
+22 −6
Original line number Diff line number Diff line
@@ -142,10 +142,15 @@ static int bru_set_format(struct v4l2_subdev *subdev,
	struct vsp1_bru *bru = to_bru(subdev);
	struct v4l2_subdev_pad_config *config;
	struct v4l2_mbus_framefmt *format;
	int ret = 0;

	mutex_lock(&bru->entity.lock);

	config = vsp1_entity_get_pad_config(&bru->entity, cfg, fmt->which);
	if (!config)
		return -EINVAL;
	if (!config) {
		ret = -EINVAL;
		goto done;
	}

	bru_try_format(bru, config, fmt->pad, &fmt->format);

@@ -174,7 +179,9 @@ static int bru_set_format(struct v4l2_subdev *subdev,
		}
	}

	return 0;
done:
	mutex_unlock(&bru->entity.lock);
	return ret;
}

static int bru_get_selection(struct v4l2_subdev *subdev,
@@ -201,7 +208,9 @@ static int bru_get_selection(struct v4l2_subdev *subdev,
		if (!config)
			return -EINVAL;

		mutex_lock(&bru->entity.lock);
		sel->r = *bru_get_compose(bru, config, sel->pad);
		mutex_unlock(&bru->entity.lock);
		return 0;

	default:
@@ -217,6 +226,7 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
	struct v4l2_subdev_pad_config *config;
	struct v4l2_mbus_framefmt *format;
	struct v4l2_rect *compose;
	int ret = 0;

	if (sel->pad == bru->entity.source_pad)
		return -EINVAL;
@@ -224,9 +234,13 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
	if (sel->target != V4L2_SEL_TGT_COMPOSE)
		return -EINVAL;

	mutex_lock(&bru->entity.lock);

	config = vsp1_entity_get_pad_config(&bru->entity, cfg, sel->which);
	if (!config)
		return -EINVAL;
	if (!config) {
		ret = -EINVAL;
		goto done;
	}

	/* The compose rectangle top left corner must be inside the output
	 * frame.
@@ -246,7 +260,9 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
	compose = bru_get_compose(bru, config, sel->pad);
	*compose = sel->r;

	return 0;
done:
	mutex_unlock(&bru->entity.lock);
	return ret;
}

static const struct v4l2_subdev_pad_ops bru_pad_ops = {
+11 −4
Original line number Diff line number Diff line
@@ -148,10 +148,15 @@ static int clu_set_format(struct v4l2_subdev *subdev,
	struct vsp1_clu *clu = to_clu(subdev);
	struct v4l2_subdev_pad_config *config;
	struct v4l2_mbus_framefmt *format;
	int ret = 0;

	mutex_lock(&clu->entity.lock);

	config = vsp1_entity_get_pad_config(&clu->entity, cfg, fmt->which);
	if (!config)
		return -EINVAL;
	if (!config) {
		ret = -EINVAL;
		goto done;
	}

	/* Default to YUV if the requested format is not supported. */
	if (fmt->format.code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
@@ -164,7 +169,7 @@ static int clu_set_format(struct v4l2_subdev *subdev,
	if (fmt->pad == CLU_PAD_SOURCE) {
		/* The CLU output format can't be modified. */
		fmt->format = *format;
		return 0;
		goto done;
	}

	format->code = fmt->format.code;
@@ -182,7 +187,9 @@ static int clu_set_format(struct v4l2_subdev *subdev,
					    CLU_PAD_SOURCE);
	*format = fmt->format;

	return 0;
done:
	mutex_unlock(&clu->entity.lock);
	return ret;
}

/* -----------------------------------------------------------------------------
+19 −3
Original line number Diff line number Diff line
@@ -51,6 +51,9 @@ void vsp1_entity_route_setup(struct vsp1_entity *source,
 * @cfg: the TRY pad configuration
 * @which: configuration selector (ACTIVE or TRY)
 *
 * When called with which set to V4L2_SUBDEV_FORMAT_ACTIVE the caller must hold
 * the entity lock to access the returned configuration.
 *
 * Return the pad configuration requested by the which argument. The TRY
 * configuration is passed explicitly to the function through the cfg argument
 * and simply returned when requested. The ACTIVE configuration comes from the
@@ -160,7 +163,9 @@ int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
	if (!config)
		return -EINVAL;

	mutex_lock(&entity->lock);
	fmt->format = *vsp1_entity_get_pad_format(entity, config, fmt->pad);
	mutex_unlock(&entity->lock);

	return 0;
}
@@ -204,8 +209,10 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev,
		if (!config)
			return -EINVAL;

		mutex_lock(&entity->lock);
		format = vsp1_entity_get_pad_format(entity, config, 0);
		code->code = format->code;
		mutex_unlock(&entity->lock);
	}

	return 0;
@@ -235,6 +242,7 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
	struct vsp1_entity *entity = to_vsp1_entity(subdev);
	struct v4l2_subdev_pad_config *config;
	struct v4l2_mbus_framefmt *format;
	int ret = 0;

	config = vsp1_entity_get_pad_config(entity, cfg, fse->which);
	if (!config)
@@ -242,8 +250,12 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,

	format = vsp1_entity_get_pad_format(entity, config, fse->pad);

	if (fse->index || fse->code != format->code)
		return -EINVAL;
	mutex_lock(&entity->lock);

	if (fse->index || fse->code != format->code) {
		ret = -EINVAL;
		goto done;
	}

	if (fse->pad == 0) {
		fse->min_width = min_width;
@@ -260,7 +272,9 @@ int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev,
		fse->max_height = format->height;
	}

	return 0;
done:
	mutex_unlock(&entity->lock);
	return ret;
}

/* -----------------------------------------------------------------------------
@@ -358,6 +372,8 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
	if (i == ARRAY_SIZE(vsp1_routes))
		return -EINVAL;

	mutex_init(&entity->lock);

	entity->vsp1 = vsp1;
	entity->source_pad = num_pads - 1;

+3 −1
Original line number Diff line number Diff line
@@ -14,7 +14,7 @@
#define __VSP1_ENTITY_H__

#include <linux/list.h>
#include <linux/spinlock.h>
#include <linux/mutex.h>

#include <media/v4l2-subdev.h>

@@ -96,6 +96,8 @@ struct vsp1_entity {

	struct v4l2_subdev subdev;
	struct v4l2_subdev_pad_config *config;

	struct mutex lock;	/* Protects the pad config */
};

static inline struct vsp1_entity *to_vsp1_entity(struct v4l2_subdev *subdev)
+11 −4
Original line number Diff line number Diff line
@@ -71,10 +71,15 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
	struct vsp1_hsit *hsit = to_hsit(subdev);
	struct v4l2_subdev_pad_config *config;
	struct v4l2_mbus_framefmt *format;
	int ret = 0;

	mutex_lock(&hsit->entity.lock);

	config = vsp1_entity_get_pad_config(&hsit->entity, cfg, fmt->which);
	if (!config)
		return -EINVAL;
	if (!config) {
		ret = -EINVAL;
		goto done;
	}

	format = vsp1_entity_get_pad_format(&hsit->entity, config, fmt->pad);

@@ -83,7 +88,7 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
		 * modified.
		 */
		fmt->format = *format;
		return 0;
		goto done;
	}

	format->code = hsit->inverse ? MEDIA_BUS_FMT_AHSV8888_1X32
@@ -104,7 +109,9 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
	format->code = hsit->inverse ? MEDIA_BUS_FMT_ARGB8888_1X32
		     : MEDIA_BUS_FMT_AHSV8888_1X32;

	return 0;
done:
	mutex_unlock(&hsit->entity.lock);
	return ret;
}

static const struct v4l2_subdev_pad_ops hsit_pad_ops = {
Loading