Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Fri, 17 Apr 2020, syzbot wrote:

> Hello,
> 
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_stop
> 
> usb 2-1: USB disconnect, device number 5
> ------------[ cut here ]------------
> usbhid 2-1:0.0: Stop while open (alloc = 1)

Okay, good.  usbhid_close() should have been called before this 
happened.  Let's try tracing the pathway; maybe this will show where it 
goes off the rails.

Alan Stern


#syz test: https://github.com/google/kasan.git 0fa84af8

Index: usb-devel/drivers/hid/usbhid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/usbhid/hid-core.c
+++ usb-devel/drivers/hid/usbhid/hid-core.c
@@ -747,6 +747,7 @@ static void usbhid_close(struct hid_devi
 		return;
 
 	hid_cancel_delayed_stuff(usbhid);
+	--hid->alan_open;
 	usb_kill_urb(usbhid->urbin);
 	usbhid->intf->needs_remote_wakeup = 0;
 }
@@ -1177,6 +1178,7 @@ static int usbhid_start(struct hid_devic
 		usbhid_set_leds(hid);
 		device_set_wakeup_enable(&dev->dev, 1);
 	}
+	++hid->alan_open;
 	return 0;
 
 fail:
@@ -1197,6 +1199,10 @@ static void usbhid_stop(struct hid_devic
 	if (WARN_ON(!usbhid))
 		return;
 
+	if (hid->alan_open > 0)
+		dev_WARN(&usbhid->intf->dev, "Stop while open = %d: %d %d %d\n",
+				hid->alan_open,
+				hid->alan1, hid->alan2, hid->alan3);
 	if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
 		clear_bit(HID_IN_POLLING, &usbhid->iofl);
 		usbhid->intf->needs_remote_wakeup = 0;
Index: usb-devel/drivers/hid/hid-input.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-input.c
+++ usb-devel/drivers/hid/hid-input.c
@@ -1960,6 +1960,7 @@ void hidinput_disconnect(struct hid_devi
 {
 	struct hid_input *hidinput, *next;
 
+	++hid->alan1;
 	hidinput_cleanup_battery(hid);
 
 	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
Index: usb-devel/drivers/input/evdev.c
===================================================================
--- usb-devel.orig/drivers/input/evdev.c
+++ usb-devel/drivers/input/evdev.c
@@ -23,6 +23,7 @@
 #include <linux/major.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 
 struct evdev {
@@ -1329,6 +1330,11 @@ static void evdev_mark_dead(struct evdev
 static void evdev_cleanup(struct evdev *evdev)
 {
 	struct input_handle *handle = &evdev->handle;
+	struct hid_device *hid;
+
+	hid = (struct hid_device *) input_get_drvdata(evdev->handle.dev);
+	if (hid)
+		++hid->alan3;
 
 	evdev_mark_dead(evdev);
 	evdev_hangup(evdev);
Index: usb-devel/drivers/input/input.c
===================================================================
--- usb-devel.orig/drivers/input/input.c
+++ usb-devel/drivers/input/input.c
@@ -23,6 +23,7 @@
 #include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 #include "input-poller.h"
 
@@ -2081,7 +2082,11 @@ static void input_cleanse_bitmasks(struc
 static void __input_unregister_device(struct input_dev *dev)
 {
 	struct input_handle *handle, *next;
+	struct hid_device *hid;
 
+	hid = (struct hid_device *) input_get_drvdata(dev);
+	if (hid)
+		++hid->alan2;
 	input_disconnect_device(dev);
 
 	mutex_lock(&input_mutex);
Index: usb-devel/include/linux/hid.h
===================================================================
--- usb-devel.orig/include/linux/hid.h
+++ usb-devel/include/linux/hid.h
@@ -618,6 +618,9 @@ struct hid_device {							/* device repo
 	struct list_head debug_list;
 	spinlock_t  debug_list_lock;
 	wait_queue_head_t debug_wait;
+
+	int alan_open;
+	int alan1, alan2, alan3;
 };
 
 #define to_hid_device(pdev) \