Commit bef4f4a8 authored by Yuchan Nam's avatar Yuchan Nam Committed by Mauro Carvalho Chehab
Browse files

media: mc, v4l2: serialize REINIT and REQBUFS with req_queue_mutex



MEDIA_REQUEST_IOC_REINIT can run concurrently with VIDIOC_REQBUFS(0)
queue teardown paths. This can race request object cleanup against vb2
queue cancellation and lead to use-after-free reports.

We already serialize request queueing against STREAMON/OFF with
req_queue_mutex. Extend that serialization to REQBUFS, and also take
the same mutex in media_request_ioctl_reinit() so REINIT is in the
same exclusion domain.

This keeps request cleanup and queue cancellation from running in
parallel for request-capable devices.

Fixes: 6093d300 ("media: vb2: keep a reference to the request until dqbuf")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarYuchan Nam <entropy1110@gmail.com>
Signed-off-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
parent ac62a200
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -192,6 +192,8 @@ static long media_request_ioctl_reinit(struct media_request *req)
	struct media_device *mdev = req->mdev;
	unsigned long flags;

	mutex_lock(&mdev->req_queue_mutex);

	spin_lock_irqsave(&req->lock, flags);
	if (req->state != MEDIA_REQUEST_STATE_IDLE &&
	    req->state != MEDIA_REQUEST_STATE_COMPLETE) {
@@ -199,6 +201,7 @@ static long media_request_ioctl_reinit(struct media_request *req)
			"request: %s not in idle or complete state, cannot reinit\n",
			req->debug_str);
		spin_unlock_irqrestore(&req->lock, flags);
		mutex_unlock(&mdev->req_queue_mutex);
		return -EBUSY;
	}
	if (req->access_count) {
@@ -206,6 +209,7 @@ static long media_request_ioctl_reinit(struct media_request *req)
			"request: %s is being accessed, cannot reinit\n",
			req->debug_str);
		spin_unlock_irqrestore(&req->lock, flags);
		mutex_unlock(&mdev->req_queue_mutex);
		return -EBUSY;
	}
	req->state = MEDIA_REQUEST_STATE_CLEANING;
@@ -216,6 +220,7 @@ static long media_request_ioctl_reinit(struct media_request *req)
	spin_lock_irqsave(&req->lock, flags);
	req->state = MEDIA_REQUEST_STATE_IDLE;
	spin_unlock_irqrestore(&req->lock, flags);
	mutex_unlock(&mdev->req_queue_mutex);

	return 0;
}
+3 −2
Original line number Diff line number Diff line
@@ -3082,13 +3082,14 @@ static long __video_do_ioctl(struct file *file,
	}

	/*
	 * We need to serialize streamon/off with queueing new requests.
	 * We need to serialize streamon/off/reqbufs with queueing new requests.
	 * These ioctls may trigger the cancellation of a streaming
	 * operation, and that should not be mixed with queueing a new
	 * request at the same time.
	 */
	if (v4l2_device_supports_requests(vfd->v4l2_dev) &&
	    (cmd == VIDIOC_STREAMON || cmd == VIDIOC_STREAMOFF)) {
	    (cmd == VIDIOC_STREAMON || cmd == VIDIOC_STREAMOFF ||
	     cmd == VIDIOC_REQBUFS)) {
		req_queue_lock = &vfd->v4l2_dev->mdev->req_queue_mutex;

		if (mutex_lock_interruptible(req_queue_lock))