Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Oliver Neukum <oneukum@suse.com>
To: syzbot <syzbot+9ca7a12fd736d93e0232@syzkaller.appspotmail.com>,
	andreyknvl@google.com, hverkuil@xs4all.nl,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	linux-usb@vger.kernel.org, mchehab@kernel.org,
	syzkaller-bugs@googlegroups.com
Subject: Re: KASAN: use-after-free Read in si470x_int_in_callback (2)
Date: Wed, 04 Dec 2019 16:03:29 +0100
Message-ID: <1575471809.30318.6.camel@suse.com> (raw)
In-Reply-To: <000000000000f47f0b0595307ddc@google.com>

Am Freitag, den 18.10.2019, 07:53 -0700 schrieb syzbot:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    22be26f7 usb-fuzzer: main usb gadget fuzzer driver
> git tree:       https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=102b65cf600000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=387eccb7ac68ec5
> dashboard link: https://syzkaller.appspot.com/bug?extid=9ca7a12fd736d93e0232
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=143b9060e00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=15d3b94b600000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+9ca7a12fd736d93e0232@syzkaller.appspotmail.com

JUST IN CASE
Final test before submission

#syz test: https://github.com/google/kasan.git 22be26f7

From ccc2a7baec5a5117216972b1c502c5a0b97de0c4 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Wed, 4 Dec 2019 13:40:19 +0100
Subject: [PATCH] si470x: fixup error handling of the interrupt URB

The error handling of the interrupt URB is not correct
in every case and assumes that low level errors
are either transient or end with a disconnect.

Starting IO to a device is not necessarily a NOP in every error
case. So we need to terminate all IO in every case of probe
failure and disconnect with absolute certainty.
We also must not retry forever in an error case.
As this is unlikely in an actual device, we just give
up.

Reported-and-tested-by: syzbot+9ca7a12fd736d93e0232@syzkaller.appspotmail.com
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/media/radio/si470x/radio-si470x-usb.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c
index fedff68d8c49..1b974c2683a6 100644
--- a/drivers/media/radio/si470x/radio-si470x-usb.c
+++ b/drivers/media/radio/si470x/radio-si470x-usb.c
@@ -370,15 +370,12 @@ static void si470x_int_in_callback(struct urb *urb)
 	unsigned char tmpbuf[3];
 
 	if (urb->status) {
-		if (urb->status == -ENOENT ||
+		if (!(urb->status == -ENOENT ||
 				urb->status == -ECONNRESET ||
-				urb->status == -ESHUTDOWN) {
-			return;
-		} else {
+				urb->status == -ESHUTDOWN))
 			dev_warn(&radio->intf->dev,
 			 "non-zero urb status (%d)\n", urb->status);
-			goto resubmit; /* Maybe we can recover. */
-		}
+		return;
 	}
 
 	/* Sometimes the device returns len 0 packets */
@@ -463,6 +460,7 @@ static void si470x_int_in_callback(struct urb *urb)
 	/* Resubmit if we're still running. */
 	if (radio->int_in_running && radio->usbdev) {
 		retval = usb_submit_urb(radio->int_in_urb, GFP_ATOMIC);
+		printk(KERN_ERR"In resubmit code path with result %d\n", retval);
 		if (retval) {
 			dev_warn(&radio->intf->dev,
 			       "resubmitting urb failed (%d)", retval);
@@ -542,6 +540,8 @@ static int si470x_start_usb(struct si470x_device *radio)
 		radio->int_in_running = 0;
 	}
 	radio->status_rssi_auto_update = radio->int_in_running;
+	if (retval < 0)
+		return retval;
 
 	/* start radio */
 	retval = si470x_start(radio);
@@ -734,7 +734,8 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 	/* start radio */
 	retval = si470x_start_usb(radio);
 	if (retval < 0)
-		goto err_buf;
+		/* the urb may be running even after an error */
+		goto err_all;
 
 	/* set initial frequency */
 	si470x_set_freq(radio, 87.5 * FREQ_MUL); /* available in all regions */
@@ -749,8 +750,7 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 
 	return 0;
 err_all:
-	usb_kill_urb(radio->int_in_urb);
-err_buf:
+	usb_poison_urb(radio->int_in_urb);
 	kfree(radio->buffer);
 err_ctrl:
 	v4l2_ctrl_handler_free(&radio->hdl);
@@ -824,7 +824,7 @@ static void si470x_usb_driver_disconnect(struct usb_interface *intf)
 	mutex_lock(&radio->lock);
 	v4l2_device_disconnect(&radio->v4l2_dev);
 	video_unregister_device(&radio->videodev);
-	usb_kill_urb(radio->int_in_urb);
+	usb_poison_urb(radio->int_in_urb);
 	usb_set_intfdata(intf, NULL);
 	mutex_unlock(&radio->lock);
 	v4l2_device_put(&radio->v4l2_dev);
-- 
2.16.4


  parent reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-18 14:53 syzbot
2019-11-18 13:44 ` Oliver Neukum
2019-11-19  9:10   ` syzbot
2019-11-20 10:32 ` Oliver Neukum
2019-11-20 23:50   ` syzbot
2019-11-21 12:00 ` Oliver Neukum
2019-11-22 10:33   ` syzbot
2019-11-22 15:35     ` Alan Stern
2019-11-22 19:00       ` Oliver Neukum
2019-11-22 20:12         ` Alan Stern
2019-11-27 10:27 ` Oliver Neukum
2019-11-27 16:30   ` syzbot
2019-11-27 18:07     ` Alan Stern
2019-11-27 20:55       ` syzbot
2019-11-27 21:11         ` Alan Stern
2019-11-28 15:19           ` Oliver Neukum
2019-11-28 17:25             ` Alan Stern
2019-11-28 10:51       ` Oliver Neukum
2019-11-28 17:33         ` Alan Stern
2019-11-28 11:10 ` Oliver Neukum
2019-11-28 13:53   ` syzbot
2019-12-04 15:03 ` Oliver Neukum [this message]
2019-12-04 18:17   ` syzbot

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1575471809.30318.6.camel@suse.com \
    --to=oneukum@suse.com \
    --cc=andreyknvl@google.com \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=syzbot+9ca7a12fd736d93e0232@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/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 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


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