From patchwork Tue Oct 1 20:18:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Stern X-Patchwork-Id: 11169619 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0593915AB for ; Tue, 1 Oct 2019 20:18:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E14CE2133F for ; Tue, 1 Oct 2019 20:18:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727066AbfJAUSz (ORCPT ); Tue, 1 Oct 2019 16:18:55 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:49192 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726068AbfJAUSz (ORCPT ); Tue, 1 Oct 2019 16:18:55 -0400 Received: (qmail 8138 invoked by uid 2102); 1 Oct 2019 16:18:54 -0400 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 1 Oct 2019 16:18:54 -0400 Date: Tue, 1 Oct 2019 16:18:54 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Hans Verkuil cc: linux-media@vger.kernel.org, USB list , Subject: [PATCH 1/2] media: usbvision: Fix invalid accesses after device disconnect Message-ID: MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org The syzbot fuzzer found two invalid-access bugs in the usbvision driver. These bugs occur when userspace keeps the device file open after the device has been disconnected and usbvision_disconnect() has set usbvision->dev to NULL: When the device file is closed, usbvision_radio_close() tries to issue a usb_set_interface() call, passing the NULL pointer as its first argument. If userspace performs a querycap ioctl call, vidioc_querycap() calls usb_make_path() with the same NULL pointer. This patch fixes the problems by making the appropriate tests beforehand. Note that vidioc_querycap() is protected by usbvision->v4l2_lock, acquired in a higher layer of the V4L2 subsystem. Reported-and-tested-by: syzbot+7fa38a608b1075dfd634@syzkaller.appspotmail.com Signed-off-by: Alan Stern CC: --- [as1919] drivers/media/usb/usbvision/usbvision-video.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) Index: usb-devel/drivers/media/usb/usbvision/usbvision-video.c =================================================================== --- usb-devel.orig/drivers/media/usb/usbvision/usbvision-video.c +++ usb-devel/drivers/media/usb/usbvision/usbvision-video.c @@ -453,6 +453,9 @@ static int vidioc_querycap(struct file * { struct usb_usbvision *usbvision = video_drvdata(file); + if (!usbvision->dev) + return -ENODEV; + strscpy(vc->driver, "USBVision", sizeof(vc->driver)); strscpy(vc->card, usbvision_device_data[usbvision->dev_model].model_string, @@ -1111,7 +1114,8 @@ static int usbvision_radio_close(struct mutex_lock(&usbvision->v4l2_lock); /* Set packet size to 0 */ usbvision->iface_alt = 0; - usb_set_interface(usbvision->dev, usbvision->iface, + if (usbvision->dev) + usb_set_interface(usbvision->dev, usbvision->iface, usbvision->iface_alt); usbvision_audio_off(usbvision); From patchwork Tue Oct 1 20:19:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Stern X-Patchwork-Id: 11169623 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B22411709 for ; Tue, 1 Oct 2019 20:19:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 992692168B for ; Tue, 1 Oct 2019 20:19:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727158AbfJAUTM (ORCPT ); Tue, 1 Oct 2019 16:19:12 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:49210 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1725851AbfJAUTM (ORCPT ); Tue, 1 Oct 2019 16:19:12 -0400 Received: (qmail 8144 invoked by uid 2102); 1 Oct 2019 16:19:11 -0400 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 1 Oct 2019 16:19:11 -0400 Date: Tue, 1 Oct 2019 16:19:11 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Hans Verkuil cc: linux-media@vger.kernel.org, USB list , Subject: [PATCH 2/2] media: usbvision: Fix races among open, close, and disconnect Message-ID: MIME-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org 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 CC: --- [as1920] drivers/media/usb/usbvision/usbvision-video.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) Index: usb-devel/drivers/media/usb/usbvision/usbvision-video.c =================================================================== --- usb-devel.orig/drivers/media/usb/usbvision/usbvision-video.c +++ usb-devel/drivers/media/usb/usbvision/usbvision-video.c @@ -314,6 +314,10 @@ static int usbvision_v4l2_open(struct fi 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 f 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; @@ -1076,6 +1082,11 @@ static int usbvision_radio_open(struct f 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; @@ -1108,6 +1119,7 @@ out: static int usbvision_radio_close(struct file *file) { struct usb_usbvision *usbvision = video_drvdata(file); + int r; PDEBUG(DBG_IO, ""); @@ -1121,9 +1133,10 @@ static int usbvision_radio_close(struct 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); @@ -1555,6 +1568,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, ""); @@ -1571,13 +1585,14 @@ static void usbvision_disconnect(struct 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);