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: 6093d3002eab ("media: vb2: keep a reference to the request until dqbuf")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchan Nam <entropy1110@gmail.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/drivers/media/mc/mc-request.c b/drivers/media/mc/mc-request.c
index c7c7d4a86c6bc55f81bded5006f853138c256f64..13e77648807c29306f2eeb0b6973121e9c301634 100644 (file)
--- a/drivers/media/mc/mc-request.c
+++ b/drivers/media/mc/mc-request.c
@@ -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;
 }

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 37d33d4a363d7eb02119b84bf368dac8e7a8bd8e..a2b650f4ec3c32a4883521f34fb51eed13c71d76 100644 (file)
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -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))