Forwarded: [PATCH] KASAN: slab-use-after-free Read in v4l2_release (3)

syzbot posted 1 patch 1 week, 3 days ago
There is a newer version of this series
Forwarded: [PATCH] KASAN: slab-use-after-free Read in v4l2_release (3)
Posted by syzbot 1 week, 3 days ago
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.

***

Subject: [PATCH] KASAN: slab-use-after-free Read in v4l2_release (3)
Author: jchuang26@m.fudan.edu.cn

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 08df884136f1c1197bab2a27814404fd329d9aac

Reported-by: syzbot+34957180b0ed2581edaf@syzkaller.appspotmail.com

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 09d8c5f8d..4da6ad1c3 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -647,6 +647,24 @@ static int sur40_input_setup_events(struct input_dev *input_dev)
 	return 0;
 }
 
+/*
+ * Final cleanup of the driver state.  This runs through v4l2_device_put()
+ * once the last reference to the v4l2_device is gone, i.e. after
+ * video_unregister_device() and after every open file descriptor of the
+ * video node has been closed.  Freeing sur40 earlier (for instance
+ * directly in sur40_disconnect()) would let v4l2_release() access the
+ * already freed video_device.
+ */
+static void sur40_v4l2_release(struct v4l2_device *v4l2_dev)
+{
+	struct sur40_state *sur40 = container_of(v4l2_dev, struct sur40_state, v4l2);
+
+	v4l2_device_unregister(&sur40->v4l2);
+	v4l2_ctrl_handler_free(&sur40->hdl);
+	kfree(sur40->bulk_in_buffer);
+	kfree(sur40);
+}
+
 /* Check candidate USB interface. */
 static int sur40_probe(struct usb_interface *interface,
 		       const struct usb_device_id *id)
@@ -727,6 +745,7 @@ static int sur40_probe(struct usb_interface *interface,
 
 	/* register the video master device */
 	snprintf(sur40->v4l2.name, sizeof(sur40->v4l2.name), "%s", DRIVER_LONG);
+	sur40->v4l2.release = sur40_v4l2_release;
 	error = v4l2_device_register(sur40->dev, &sur40->v4l2);
 	if (error) {
 		dev_err(&interface->dev,
@@ -778,7 +797,6 @@ static int sur40_probe(struct usb_interface *interface,
 	if (sur40->hdl.error) {
 		dev_err(&interface->dev,
 			"Unable to register video controls.");
-		v4l2_ctrl_handler_free(&sur40->hdl);
 		error = sur40->hdl.error;
 		goto err_unreg_v4l2;
 	}
@@ -787,7 +805,7 @@ static int sur40_probe(struct usb_interface *interface,
 	if (error) {
 		dev_err(&interface->dev,
 			"Unable to register video subdevice.");
-		goto err_free_ctrl;
+		goto err_unreg_v4l2;
 	}
 
 	/* register the polled input device */
@@ -806,10 +824,17 @@ static int sur40_probe(struct usb_interface *interface,
 
 err_unreg_video:
 	video_unregister_device(&sur40->vdev);
-err_free_ctrl:
-	v4l2_ctrl_handler_free(&sur40->hdl);
 err_unreg_v4l2:
-	v4l2_device_unregister(&sur40->v4l2);
+	/*
+	 * Drop the reference taken above by v4l2_device_register().  The
+	 * last reference releases sur40 via sur40_v4l2_release(), which
+	 * also frees the control handler and the bulk buffer.
+	 */
+	input_free_device(input);
+	v4l2_device_put(&sur40->v4l2);
+
+	return error;
+
 err_free_buffer:
 	kfree(sur40->bulk_in_buffer);
 err_free_input:
@@ -827,15 +852,19 @@ static void sur40_disconnect(struct usb_interface *interface)
 
 	input_unregister_device(sur40->input);
 
-	v4l2_ctrl_handler_free(&sur40->hdl);
 	video_unregister_device(&sur40->vdev);
 	v4l2_device_unregister(&sur40->v4l2);
 
-	kfree(sur40->bulk_in_buffer);
-	kfree(sur40);
-
 	usb_set_intfdata(interface, NULL);
 	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);
+
+	/*
+	 * Release the reference taken by v4l2_device_register().  If the
+	 * video node is still open, sur40_v4l2_release() is deferred until
+	 * the last file descriptor is closed, so sur40 stays valid for
+	 * v4l2_release().
+	 */
+	v4l2_device_put(&sur40->v4l2);
 }
 
 /*