Stable Archive on lore.kernel.org
 help / color / Atom feed
* [git:media_tree/master] media: usbvision: Fix races among open, close, and disconnect
@ 2019-10-10 10:22 Mauro Carvalho Chehab
  0 siblings, 0 replies; 2+ messages in thread
From: Mauro Carvalho Chehab @ 2019-10-10 10:22 UTC (permalink / raw)
  To: linuxtv-commits; +Cc: stable, Alan Stern, Hans Verkuil

This is an automatic generated email to let you know that the following patch were queued:

Subject: media: usbvision: Fix races among open, close, and disconnect
Author:  Alan Stern <stern@rowland.harvard.edu>
Date:    Mon Oct 7 12:09:53 2019 -0300

Visual inspection of the usbvision driver shows that it suffers from
three races between its open, close, and disconnect handlers.  In
particular, the driver is careful to update its usbvision->user and
usbvision->remove_pending flags while holding the private mutex, but:

	usbvision_v4l2_close() and usbvision_radio_close() don't hold
	the mutex while they check the value of
	usbvision->remove_pending;

	usbvision_disconnect() doesn't hold the mutex while checking
	the value of usbvision->user; and

	also, usbvision_v4l2_open() and usbvision_radio_open() don't
	check whether the device has been unplugged before allowing
	the user to open the device files.

Each of these can potentially lead to usbvision_release() being called
twice and use-after-free errors.

This patch fixes the races by reading the flags while the mutex is
still held and checking for pending removes before allowing an open to
succeed.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: <stable@vger.kernel.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

 drivers/media/usb/usbvision/usbvision-video.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

---

diff --git a/drivers/media/usb/usbvision/usbvision-video.c b/drivers/media/usb/usbvision/usbvision-video.c
index 62dec73aec6e..93d36aab824f 100644
--- a/drivers/media/usb/usbvision/usbvision-video.c
+++ b/drivers/media/usb/usbvision/usbvision-video.c
@@ -314,6 +314,10 @@ static int usbvision_v4l2_open(struct file *file)
 	if (mutex_lock_interruptible(&usbvision->v4l2_lock))
 		return -ERESTARTSYS;
 
+	if (usbvision->remove_pending) {
+		err_code = -ENODEV;
+		goto unlock;
+	}
 	if (usbvision->user) {
 		err_code = -EBUSY;
 	} else {
@@ -377,6 +381,7 @@ unlock:
 static int usbvision_v4l2_close(struct file *file)
 {
 	struct usb_usbvision *usbvision = video_drvdata(file);
+	int r;
 
 	PDEBUG(DBG_IO, "close");
 
@@ -391,9 +396,10 @@ static int usbvision_v4l2_close(struct file *file)
 	usbvision_scratch_free(usbvision);
 
 	usbvision->user--;
+	r = usbvision->remove_pending;
 	mutex_unlock(&usbvision->v4l2_lock);
 
-	if (usbvision->remove_pending) {
+	if (r) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
 		return 0;
@@ -1064,6 +1070,11 @@ static int usbvision_radio_open(struct file *file)
 
 	if (mutex_lock_interruptible(&usbvision->v4l2_lock))
 		return -ERESTARTSYS;
+
+	if (usbvision->remove_pending) {
+		err_code = -ENODEV;
+		goto out;
+	}
 	err_code = v4l2_fh_open(file);
 	if (err_code)
 		goto out;
@@ -1096,6 +1107,7 @@ out:
 static int usbvision_radio_close(struct file *file)
 {
 	struct usb_usbvision *usbvision = video_drvdata(file);
+	int r;
 
 	PDEBUG(DBG_IO, "");
 
@@ -1109,9 +1121,10 @@ static int usbvision_radio_close(struct file *file)
 	usbvision_audio_off(usbvision);
 	usbvision->radio = 0;
 	usbvision->user--;
+	r = usbvision->remove_pending;
 	mutex_unlock(&usbvision->v4l2_lock);
 
-	if (usbvision->remove_pending) {
+	if (r) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		v4l2_fh_release(file);
 		usbvision_release(usbvision);
@@ -1543,6 +1556,7 @@ err_usb:
 static void usbvision_disconnect(struct usb_interface *intf)
 {
 	struct usb_usbvision *usbvision = to_usbvision(usb_get_intfdata(intf));
+	int u;
 
 	PDEBUG(DBG_PROBE, "");
 
@@ -1559,13 +1573,14 @@ static void usbvision_disconnect(struct usb_interface *intf)
 	v4l2_device_disconnect(&usbvision->v4l2_dev);
 	usbvision_i2c_unregister(usbvision);
 	usbvision->remove_pending = 1;	/* Now all ISO data will be ignored */
+	u = usbvision->user;
 
 	usb_put_dev(usbvision->dev);
 	usbvision->dev = NULL;	/* USB device is no more */
 
 	mutex_unlock(&usbvision->v4l2_lock);
 
-	if (usbvision->user) {
+	if (u) {
 		printk(KERN_INFO "%s: In use, disconnect pending\n",
 		       __func__);
 		wake_up_interruptible(&usbvision->wait_frame);

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [git:media_tree/master] media: usbvision: Fix races among open, close, and disconnect
       [not found] <20191014035427.DCD2B20882@mail.kernel.org>
@ 2019-10-15 19:45 ` Alan Stern
  0 siblings, 0 replies; 2+ messages in thread
From: Alan Stern @ 2019-10-15 19:45 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Mauro Carvalho Chehab, linuxtv-commits, stable

On Mon, 14 Oct 2019, Sasha Levin wrote:

> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v5.3.5, v5.2.20, v4.19.78, v4.14.148, v4.9.196, v4.4.196.
> 
> v5.3.5: Build OK!
> v5.2.20: Build OK!
> v4.19.78: Build OK!
> v4.14.148: Build OK!
> v4.9.196: Build OK!
> v4.4.196: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?

As far as I'm concerned, it's probably not worth the effort of
backporting this to 4.4.y.

Alan Stern


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 10:22 [git:media_tree/master] media: usbvision: Fix races among open, close, and disconnect Mauro Carvalho Chehab
     [not found] <20191014035427.DCD2B20882@mail.kernel.org>
2019-10-15 19:45 ` Alan Stern

Stable Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://lore.kernel.org/stable \
		stable@vger.kernel.org stable@archiver.kernel.org
	public-inbox-index stable

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.stable


AGPL code for this site: git clone https://public-inbox.org/ public-inbox