Commit af69562a authored by Hans de Goede's avatar Hans de Goede Committed by Mauro Carvalho Chehab
Browse files

media: atomisp: Remove the ACC device node 



The ACC /dev/video# device node uses a struct video_device embedded in
an atomisp_acc_pipe struct instead of in an atomisp_video_pipe struct.
Yet it uses the same file-ops and ioctl-ops even though it does not have
a videobuf queue, which makes e.g. the mmap fop nonsense.

Worse the only file-ops / ioctls which differentiate between
the 2 types and correctly do container_of on the right type
are the open/release fops and the vidioc_default handler.

The mmap and poll fops and *all* other ioctl handlers unconditionally
do container_of on the passed in struct video_device blindly assuming
they are dealing with the one embedded in the atomisp_video_pipe struct.

This makes it trivial for userspace to cause all sort of undefined behavior
by calling mmap, poll or the other ioctls on the ACC device node!

Presumably the use of the ACC device node was to allow making the special
ioctls to load custom programs while the other /dev/video# nodes were
already open, since the /dev/video# nodes can currently all be opened
only once (which needs to be fixed).

commit 4bbca788 ("media: atomisp: remove private acceleration ioctls")
has removed the custom ATOMISP_ACC_* ioctls, so there no longer is any
reason to keep the ACC device node.

As explained above its presence can easily cause the kernel to crash,
so remove the ACC device node and the code for handling it.

Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
parent 79adb947

drivers/staging/media/atomisp/pci/atomisp_cmd.c

+0 −9
Original line number Diff line number Diff line
@@ -98,15 +98,6 @@ struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev) 
	       container_of(dev, struct atomisp_video_pipe, vdev);

}



/*

 * get struct atomisp_acc_pipe from v4l2 video_device

 */

struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct video_device *dev)

{

	return (struct atomisp_acc_pipe *)

	       container_of(dev, struct atomisp_acc_pipe, vdev);

}



static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)

{

	struct v4l2_subdev_frame_interval fi = { 0 };

drivers/staging/media/atomisp/pci/atomisp_cmd.h

+0 −1
Original line number Diff line number Diff line
@@ -54,7 +54,6 @@ void dump_sp_dmem(struct atomisp_device *isp, unsigned int addr, 
		  unsigned int size);

struct camera_mipi_info *atomisp_to_sensor_mipi_info(struct v4l2_subdev *sd);

struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev);

struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct video_device *dev);

int atomisp_reset(struct atomisp_device *isp);

void atomisp_flush_bufs_and_wakeup(struct atomisp_sub_device *asd);

void atomisp_clear_css_buffer_counters(struct atomisp_sub_device *asd);

drivers/staging/media/atomisp/pci/atomisp_fops.c

+15 −54
Original line number Diff line number Diff line
@@ -684,8 +684,7 @@ static unsigned int atomisp_subdev_users(struct atomisp_sub_device *asd) 
	return asd->video_out_preview.users +

	       asd->video_out_vf.users +

	       asd->video_out_capture.users +

	       asd->video_out_video_capture.users +

	       asd->video_acc.users;

	       asd->video_out_video_capture.users;

}



unsigned int atomisp_dev_users(struct atomisp_device *isp)
@@ -702,10 +701,8 @@ static int atomisp_open(struct file *file) 
{

	struct video_device *vdev = video_devdata(file);

	struct atomisp_device *isp = video_get_drvdata(vdev);

	struct atomisp_video_pipe *pipe = NULL;

	struct atomisp_acc_pipe *acc_pipe = NULL;

	struct atomisp_sub_device *asd;

	bool acc_node = false;

	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);

	struct atomisp_sub_device *asd = pipe->asd;

	int ret;



	dev_dbg(isp->dev, "open device %s\n", vdev->name);
@@ -736,14 +733,6 @@ static int atomisp_open(struct file *file) 


	rt_mutex_lock(&isp->mutex);



	acc_node = !strcmp(vdev->name, "ATOMISP ISP ACC");

	if (acc_node) {

		acc_pipe = atomisp_to_acc_pipe(vdev);

		asd = acc_pipe->asd;

	} else {

		pipe = atomisp_to_video_pipe(vdev);

		asd = pipe->asd;

	}

	asd->subdev.devnode = vdev;

	/* Deferred firmware loading case. */

	if (isp->css_env.isp_css_fw.bytes == 0) {
@@ -765,14 +754,6 @@ static int atomisp_open(struct file *file) 
		isp->css_env.isp_css_fw.data = NULL;

	}



	if (acc_node && acc_pipe->users) {

		dev_dbg(isp->dev, "acc node already opened\n");

		rt_mutex_unlock(&isp->mutex);

		return -EBUSY;

	} else if (acc_node) {

		goto dev_init;

	}



	if (!isp->input_cnt) {

		dev_err(isp->dev, "no camera attached\n");

		ret = -EINVAL;
@@ -792,7 +773,6 @@ static int atomisp_open(struct file *file) 
	if (ret)

		goto error;



dev_init:

	if (atomisp_dev_users(isp)) {

		dev_dbg(isp->dev, "skip init isp in open\n");

		goto init_subdev;
@@ -827,15 +807,10 @@ static int atomisp_open(struct file *file) 
	atomisp_subdev_init_struct(asd);



done:



	if (acc_node)

		acc_pipe->users++;

	else

	pipe->users++;

	rt_mutex_unlock(&isp->mutex);



	/* Ensure that a mode is set */

	if (!acc_node)

	v4l2_ctrl_s_ctrl(asd->run_mode, pipe->default_run_mode);



	return 0;
@@ -852,10 +827,8 @@ static int atomisp_release(struct file *file) 
{

	struct video_device *vdev = video_devdata(file);

	struct atomisp_device *isp = video_get_drvdata(vdev);

	struct atomisp_video_pipe *pipe;

	struct atomisp_acc_pipe *acc_pipe;

	struct atomisp_sub_device *asd;

	bool acc_node;

	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);

	struct atomisp_sub_device *asd = pipe->asd;

	struct v4l2_requestbuffers req;

	struct v4l2_subdev_fh fh;

	struct v4l2_rect clear_compose = {0};
@@ -871,19 +844,9 @@ static int atomisp_release(struct file *file) 
	rt_mutex_lock(&isp->mutex);



	dev_dbg(isp->dev, "release device %s\n", vdev->name);

	acc_node = !strcmp(vdev->name, "ATOMISP ISP ACC");

	if (acc_node) {

		acc_pipe = atomisp_to_acc_pipe(vdev);

		asd = acc_pipe->asd;

	} else {

		pipe = atomisp_to_video_pipe(vdev);

		asd = pipe->asd;

	}



	asd->subdev.devnode = vdev;

	if (acc_node) {

		acc_pipe->users--;

		goto subdev_uninit;

	}



	pipe->users--;



	if (pipe->capq.streaming)
@@ -921,7 +884,7 @@ static int atomisp_release(struct file *file) 
					V4L2_SUBDEV_FORMAT_ACTIVE,

					ATOMISP_SUBDEV_PAD_SINK, &isp_sink_fmt);

	}

subdev_uninit:



	if (atomisp_subdev_users(asd))

		goto done;
@@ -956,13 +919,11 @@ static int atomisp_release(struct file *file) 
		dev_err(isp->dev, "Failed to power off device\n");



done:

	if (!acc_node) {

	atomisp_subdev_set_selection(&asd->subdev, fh.state,

				     V4L2_SUBDEV_FORMAT_ACTIVE,

				     atomisp_subdev_source_pad(vdev),

				     V4L2_SEL_TGT_COMPOSE, 0,

				     &clear_compose);

	}

	rt_mutex_unlock(&isp->mutex);

	mutex_unlock(&isp->streamoff_mutex);

drivers/staging/media/atomisp/pci/atomisp_ioctl.c

+1 −8
Original line number Diff line number Diff line
@@ -2773,17 +2773,10 @@ static long atomisp_vidioc_default(struct file *file, void *fh, 
{

	struct video_device *vdev = video_devdata(file);

	struct atomisp_device *isp = video_get_drvdata(vdev);

	struct atomisp_sub_device *asd;

	struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;

	struct v4l2_subdev *motor;

	bool acc_node;

	int err;



	acc_node = !strcmp(vdev->name, "ATOMISP ISP ACC");

	if (acc_node)

		asd = atomisp_to_acc_pipe(vdev)->asd;

	else

		asd = atomisp_to_video_pipe(vdev)->asd;



	if (!IS_ISP2401)

		motor = isp->inputs[asd->input_curr].motor;

	else

drivers/staging/media/atomisp/pci/atomisp_subdev.c

+0 −18
Original line number Diff line number Diff line
@@ -1075,13 +1075,6 @@ static void atomisp_init_subdev_pipe(struct atomisp_sub_device *asd, 
	       sizeof(struct atomisp_css_params_with_list *));

}



static void atomisp_init_acc_pipe(struct atomisp_sub_device *asd,

				  struct atomisp_acc_pipe *pipe)

{

	pipe->asd = asd;

	pipe->isp = asd->isp;

}



/*

 * isp_subdev_init_entities - Initialize V4L2 subdev and media entity

 * @asd: ISP CCDC module
@@ -1137,8 +1130,6 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd) 
	atomisp_init_subdev_pipe(asd, &asd->video_out_video_capture,

				 V4L2_BUF_TYPE_VIDEO_CAPTURE);



	atomisp_init_acc_pipe(asd, &asd->video_acc);



	ret = atomisp_video_init(&asd->video_out_capture, "CAPTURE",

				 ATOMISP_RUN_MODE_STILL_CAPTURE);

	if (ret < 0)
@@ -1159,8 +1150,6 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd) 
	if (ret < 0)

		return ret;



	atomisp_acc_init(&asd->video_acc, "ACC");



	ret = v4l2_ctrl_handler_init(&asd->ctrl_handler, 1);

	if (ret)

		return ret;
@@ -1290,7 +1279,6 @@ void atomisp_subdev_unregister_entities(struct atomisp_sub_device *asd) 
	atomisp_video_unregister(&asd->video_out_vf);

	atomisp_video_unregister(&asd->video_out_capture);

	atomisp_video_unregister(&asd->video_out_video_capture);

	atomisp_acc_unregister(&asd->video_acc);

}



int atomisp_subdev_register_entities(struct atomisp_sub_device *asd,
@@ -1336,12 +1324,6 @@ int atomisp_subdev_register_entities(struct atomisp_sub_device *asd, 
				    VFL_TYPE_VIDEO, -1);

	if (ret < 0)

		goto error;

	asd->video_acc.vdev.v4l2_dev = vdev;

	asd->video_acc.vdev.device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;

	ret = video_register_device(&asd->video_acc.vdev,

				    VFL_TYPE_VIDEO, -1);

	if (ret < 0)

		goto error;



	return 0;