vb2: check if vb2_fop_write/read is allowed

PATCH In review Hans Verkuil 2 participants 2 messages Mar 3, 2015 Media (V4L/DVB)

Share & tools

Hans Verkuil

author

2015-03-03 11:24 UTC #0 lore raw

PATCH 1/1 vb2: check if vb2_fop_write/read is allowed

Return -EINVAL if read() or write() is not supported by the queue. This makes it possible to provide both vb2_fop_read and vb2_fop_write in a struct v4l2_file_operations since the vb2_fop_* function will check if the file operation is allowed.

A similar check exists in __vb2_init_fileio() which is called from __vb2_perform_fileio(), but that check is only done if no file I/O is active. So the sequence of read() followed by write() would be allowed, which is obviously a bug.

In addition, vb2_fop_write/read should always return -EINVAL if the operation is not allowed, and by putting the check in the lower levels of the code it is possible that other error codes are returned (EBUSY or ERESTARTSYS).

All these issues are avoided by just doing a quick explicit check.

Signed-off-by Hans Verkuil <[email protected]>

drivers/media/v4l2-core/videobuf2-core.c +4 -0

index bc08a82..167c1d9 100644
@@ -3416,6 +3416,8 @@ ssize_t vb2_fop_write(struct file *file, const char __user *buf,
 	struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
 	int err = -EBUSY;
 
+	if (!(vdev->queue->io_modes & VB2_WRITE))
+		return -EINVAL;
 	if (lock && mutex_lock_interruptible(lock))
 		return -ERESTARTSYS;
 	if (vb2_queue_is_busy(vdev, file))

@@ -3438,6 +3440,8 @@ ssize_t vb2_fop_read(struct file *file, char __user *buf,
 	struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock;
 	int err = -EBUSY;
 
+	if (!(vdev->queue->io_modes & VB2_READ))
+		return -EINVAL;
 	if (lock && mutex_lock_interruptible(lock))
 		return -ERESTARTSYS;
 	if (vb2_queue_is_busy(vdev, file))

Sakari Ailus

Acked-by on v1 1/1

2015-03-03 22:19 UTC #1 lore raw

Quoted 10 lines · Hans Verkuil

Return -EINVAL if read() or write() is not supported by the queue. This makes it possible to provide both vb2_fop_read and vb2_fop_write in a struct v4l2_file_operations since the vb2_fop_* function will check if the file operation is allowed.
A similar check exists in __vb2_init_fileio() which is called from __vb2_perform_fileio(), but that check is only done if no file I/O is active. So the sequence of read() followed by write() would be allowed, which is obviously a bug.

Quoted 8 lines

In addition, vb2_fop_write/read should always return -EINVAL if the operation is not allowed, and by putting the check in the lower levels of the code it is possible that other error codes are returned (EBUSY or ERESTARTSYS).
All these issues are avoided by just doing a quick explicit check.
Signed-off-by Hans Verkuil <[email protected]>

Acked-by Sakari Ailus <[email protected]>

Signature · 2 lines

Sakari Ailus
e-mail: [email protected]	XMPP: [email protected]