Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f45813e8 authored by Arun Menon's avatar Arun Menon
Browse files

msm: vidc: fix a race condition in cluster construction



At start of video session, when the super cluster is
constructed, it refers to a global static variable.
If we have multiple instances running simultaneously,
there exists a race condition, causing the instance
to register wrong cluster. Later on when a control
gets set in encoder, if the control is not found in
the cluster, we hit a BUG_ON, crashing the device.

Change-Id: Ib4ee64d684d903bb4f22310a1b34619728405649
Signed-off-by: default avatarArun Menon <avmenon@codeaurora.org>
parent 54f3fb1b
Loading
Loading
Loading
Loading
+19 −5
Original line number Diff line number Diff line
@@ -2142,17 +2142,18 @@ int msm_vdec_g_ctrl(struct msm_vidc_inst *inst, struct v4l2_control *ctrl)
	return v4l2_g_ctrl(&inst->ctrl_handler, ctrl);
}

static struct v4l2_ctrl **get_super_cluster(int *size)
static struct v4l2_ctrl **get_super_cluster(struct msm_vidc_inst *inst,
				int *size)
{
	int c = 0, sz = 0;
	struct v4l2_ctrl **cluster = kmalloc(sizeof(struct v4l2_ctrl *) *
			NUM_CTRLS, GFP_KERNEL);

	if (!size || !cluster)
	if (!size || !cluster || !inst)
		return NULL;

	for (c = 0; c < NUM_CTRLS; c++)
		cluster[sz++] = msm_vdec_ctrls[c].priv;
		cluster[sz++] = inst->ctrls[c];

	*size = sz;
	return cluster;
@@ -2165,6 +2166,18 @@ int msm_vdec_ctrl_init(struct msm_vidc_inst *inst)
	int ret_val = 0;
	int cluster_size = 0;

	if (!inst) {
		dprintk(VIDC_ERR, "%s - invalid input\n", __func__);
		return -EINVAL;
	}

	inst->ctrls = kzalloc(sizeof(struct v4l2_ctrl *) * NUM_CTRLS,
				GFP_KERNEL);
	if (!inst->ctrls) {
		dprintk(VIDC_ERR, "%s - failed to allocate ctrl\n", __func__);
		return -ENOMEM;
	}

	ret_val = v4l2_ctrl_handler_init(&inst->ctrl_handler, NUM_CTRLS);

	if (ret_val) {
@@ -2237,11 +2250,11 @@ int msm_vdec_ctrl_init(struct msm_vidc_inst *inst)
			return ret_val;
		}

		msm_vdec_ctrls[idx].priv = ctrl;
		inst->ctrls[idx] = ctrl;
	}

	/* Construct a super cluster of all controls */
	inst->cluster = get_super_cluster(&cluster_size);
	inst->cluster = get_super_cluster(inst, &cluster_size);
	if (!inst->cluster || !cluster_size) {
		dprintk(VIDC_WARN,
				"Failed to setup super cluster\n");
@@ -2255,6 +2268,7 @@ int msm_vdec_ctrl_init(struct msm_vidc_inst *inst)

int msm_vdec_ctrl_deinit(struct msm_vidc_inst *inst)
{
	kfree(inst->ctrls);
	kfree(inst->cluster);
	v4l2_ctrl_handler_free(&inst->ctrl_handler);

+19 −24
Original line number Diff line number Diff line
@@ -2403,15 +2403,6 @@ static int try_set_ctrl(struct msm_vidc_inst *inst, struct v4l2_ctrl *ctrl)
	return rc;
}

static struct v4l2_ctrl *get_cluster_from_id(int id)
{
	int c;
	for (c = 0; c < ARRAY_SIZE(msm_venc_ctrls); ++c)
		if (msm_venc_ctrls[c].id == id)
			return (struct v4l2_ctrl *)msm_venc_ctrls[c].priv;
	return NULL;
}

static int try_set_ext_ctrl(struct msm_vidc_inst *inst,
	struct v4l2_ext_controls *ctrl)
{
@@ -2420,7 +2411,6 @@ static int try_set_ext_ctrl(struct msm_vidc_inst *inst,
	struct hfi_device *hdev;
	struct hal_ltrmode ltrmode;
	struct hal_vc1e_perf_cfg_type search_range = { {0} };
	struct v4l2_ctrl *cluster;
	u32 property_id = 0;
	void *pdata = NULL;
	struct msm_vidc_core_capability *cap = NULL;
@@ -2431,14 +2421,6 @@ static int try_set_ext_ctrl(struct msm_vidc_inst *inst,
		return -EINVAL;
	}

	cluster = get_cluster_from_id(ctrl->controls[0].id);

	if (!cluster) {
		dprintk(VIDC_ERR, "Invalid Ctrl returned for id: %x\n",
			ctrl->controls[0].id);
		return -EINVAL;
	}

	hdev = inst->core->device;
	cap = &inst->capability;

@@ -3248,17 +3230,18 @@ int msm_venc_streamoff(struct msm_vidc_inst *inst, enum v4l2_buf_type i)
	return rc;
}

static struct v4l2_ctrl **get_super_cluster(int *size)
static struct v4l2_ctrl **get_super_cluster(struct msm_vidc_inst *inst,
				int *size)
{
	int c = 0, sz = 0;
	struct v4l2_ctrl **cluster = kmalloc(sizeof(struct v4l2_ctrl *) *
			NUM_CTRLS, GFP_KERNEL);

	if (!size || !cluster)
	if (!size || !cluster || !inst)
		return NULL;

	for (c = 0; c < NUM_CTRLS; c++)
		cluster[sz++] = msm_venc_ctrls[c].priv;
		cluster[sz++] =  inst->ctrls[c];

	*size = sz;
	return cluster;
@@ -3266,12 +3249,23 @@ static struct v4l2_ctrl **get_super_cluster(int *size)

int msm_venc_ctrl_init(struct msm_vidc_inst *inst)
{

	int idx = 0;
	struct v4l2_ctrl_config ctrl_cfg = {0};
	int ret_val = 0;
	int cluster_size = 0;

	if (!inst) {
		dprintk(VIDC_ERR, "%s - invalid input\n", __func__);
		return -EINVAL;
	}

	inst->ctrls = kzalloc(sizeof(struct v4l2_ctrl *) * NUM_CTRLS,
				GFP_KERNEL);
	if (!inst->ctrls) {
		dprintk(VIDC_ERR, "%s - failed to allocate ctrl\n", __func__);
		return -ENOMEM;
	}

	ret_val = v4l2_ctrl_handler_init(&inst->ctrl_handler, NUM_CTRLS);
	if (ret_val) {
		dprintk(VIDC_ERR, "CTRL ERR: Control handler init failed, %d\n",
@@ -3326,11 +3320,11 @@ int msm_venc_ctrl_init(struct msm_vidc_inst *inst)
			return ret_val;
		}

		msm_venc_ctrls[idx].priv = ctrl;
		inst->ctrls[idx] = ctrl;
	}

	/* Construct a super cluster of all controls */
	inst->cluster = get_super_cluster(&cluster_size);
	inst->cluster = get_super_cluster(inst, &cluster_size);
	if (!inst->cluster || !cluster_size) {
		dprintk(VIDC_WARN,
				"Failed to setup super cluster\n");
@@ -3344,6 +3338,7 @@ int msm_venc_ctrl_init(struct msm_vidc_inst *inst)

int msm_venc_ctrl_deinit(struct msm_vidc_inst *inst)
{
	kfree(inst->ctrls);
	kfree(inst->cluster);
	v4l2_ctrl_handler_free(&inst->ctrl_handler);

+1 −1
Original line number Diff line number Diff line
@@ -249,6 +249,7 @@ struct msm_vidc_inst {
	struct list_head registered_bufs;
	bool map_output_buffer;
	atomic_t get_seq_hdr_cnt;
	struct v4l2_ctrl **ctrls;
};

extern struct msm_vidc_drv *vidc_driver;
@@ -269,7 +270,6 @@ struct msm_vidc_ctrl {
	u32 menu_skip_mask;
	u32 flags;
	const char * const *qmenu;
	struct v4l2_ctrl *priv;
};

void handle_cmd_response(enum command_response cmd, void *data);