From c1493b2ed26627ce900ad784a07e3de33cdddacb Mon Sep 17 00:00:00 2001
From: Alexander Boettcher <alexander.boettcher@genode-labs.com>
Date: Sun, 5 Nov 2017 16:36:14 +0100
Subject: [PATCH] usb: avoid pagefault during session destruction

due to pointer to object allocated in context of the session object.

Fixes #2565
---
 repos/dde_linux/src/lib/usb/raw/raw.cc | 52 +++++++++++++++++++-------
 repos/os/run/usb_block.run             |  4 +-
 repos/os/src/drivers/usb_block/main.cc | 16 ++++++--
 3 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/repos/dde_linux/src/lib/usb/raw/raw.cc b/repos/dde_linux/src/lib/usb/raw/raw.cc
index b6fd15c58..015a333e1 100644
--- a/repos/dde_linux/src/lib/usb/raw/raw.cc
+++ b/repos/dde_linux/src/lib/usb/raw/raw.cc
@@ -114,7 +114,7 @@ struct Device : List<Device>::Element
  * Handle packet stream request, this way the entrypoint always returns to it's
  * server loop
  */
-class Usb::Worker
+class Usb::Worker : public Genode::Weak_object<Usb::Worker>
 {
 	private:
 
@@ -205,10 +205,27 @@ class Usb::Worker
 		 */
 		struct Complete_data
 		{
-			Worker            *worker;
+			Weak_ptr<Worker>   worker;
 			Packet_descriptor  packet;
+
+			Complete_data(Weak_ptr<Worker> &w, Packet_descriptor &p)
+			: worker(w), packet(p) { }
 		};
 
+		Complete_data * alloc_complete_data(Packet_descriptor &p)
+		{
+			void * data = kmalloc(sizeof(Complete_data), GFP_KERNEL);
+			construct_at<Complete_data>(data, this->weak_ptr(), p);
+			return reinterpret_cast<Complete_data *>(data);
+		}
+
+		static void free_complete_data(Complete_data *data)
+		{
+			data->packet.~Packet_descriptor();
+			data->worker.~Weak_ptr<Worker>();
+			kfree (data);
+		}
+
 		void _async_finish(Packet_descriptor &p, urb *urb, bool read)
 		{
 			if (urb->status == 0) {
@@ -231,9 +248,15 @@ class Usb::Worker
 		{
 			Complete_data *data = (Complete_data *)urb->context;
 
-			data->worker->_async_finish(data->packet, urb,
-			                            !!(data->packet.transfer.ep & USB_DIR_IN));
-			kfree (data);
+			{
+				Locked_ptr<Worker> worker(data->worker);
+
+				if (worker.valid())
+					worker->_async_finish(data->packet, urb,
+					                      !!(data->packet.transfer.ep & USB_DIR_IN));
+			}
+
+			free_complete_data(data);
 			dma_free(urb->transfer_buffer);
 			usb_free_urb(urb);
 		}
@@ -261,9 +284,7 @@ class Usb::Worker
 				return false;
 			}
 
-			Complete_data *data = (Complete_data *)kmalloc(sizeof(Complete_data), GFP_KERNEL);
-			data->packet   = p;
-			data->worker   = this;
+			Complete_data *data = alloc_complete_data(p);
 
 			usb_fill_bulk_urb(bulk_urb, _device->udev, pipe, buf, p.size(),
 			                 _async_complete, data);
@@ -272,7 +293,8 @@ class Usb::Worker
 			if (ret != 0) {
 				error("Failed to submit URB, error: ", ret);
 				p.error = Usb::Packet_descriptor::SUBMIT_ERROR;
-				kfree(data);
+
+				free_complete_data(data);
 				usb_free_urb(bulk_urb);
 				dma_free(buf);
 				return false;
@@ -304,9 +326,7 @@ class Usb::Worker
 				return false;
 			}
 
-			Complete_data *data = (Complete_data *)kmalloc(sizeof(Complete_data), GFP_KERNEL);
-			data->packet   = p;
-			data->worker   = this;
+			Complete_data *data = alloc_complete_data(p);
 
 			int polling_interval;
 
@@ -326,7 +346,8 @@ class Usb::Worker
 			if (ret != 0) {
 				error("Failed to submit URB, error: ", ret);
 				p.error = Usb::Packet_descriptor::SUBMIT_ERROR;
-				kfree(data);
+
+				free_complete_data(data);
 				usb_free_urb(irq_urb);
 				dma_free(buf);
 				return false;
@@ -485,6 +506,11 @@ class Usb::Worker
 		: _sink(sink)
 		{ }
 
+		~Worker()
+		{
+			Weak_object<Worker>::lock_for_destruction();
+		}
+
 		void start()
 		{
 			if (!_task) {
diff --git a/repos/os/run/usb_block.run b/repos/os/run/usb_block.run
index f26b7a9ff..8d4c6e4cf 100644
--- a/repos/os/run/usb_block.run
+++ b/repos/os/run/usb_block.run
@@ -45,7 +45,9 @@ set config {
 	</parent-provides>
 	<default-route>
 		<any-service> <parent/> <any-child/> </any-service>
-	</default-route>}
+	</default-route>
+
+	<default caps="100"/>}
 
 append_platform_drv_config
 
diff --git a/repos/os/src/drivers/usb_block/main.cc b/repos/os/src/drivers/usb_block/main.cc
index 7b51d0c34..db7870696 100644
--- a/repos/os/src/drivers/usb_block/main.cc
+++ b/repos/os/src/drivers/usb_block/main.cc
@@ -187,7 +187,7 @@ struct Usb::Block_driver : Usb::Completion,
 			}
 
 			if (!p.succeded) {
-				Genode::error("init complete error: packet not succeded");
+				Genode::error("init complete error: packet not succeeded");
 				iface.release(p);
 				return;
 			}
@@ -720,6 +720,12 @@ struct Usb::Block_driver : Usb::Completion,
 		/* USB device gets initialized by handle_state_change() */
 	}
 
+	~Block_driver()
+	{
+		Interface &iface = device.interface(active_interface);
+		iface.release();
+	}
+
 	/**
 	 * Send CBW
 	 */
@@ -812,9 +818,13 @@ struct Usb::Main
 			driver = new (&alloc) Usb::Block_driver(env, alloc, sigh);
 		}
 
-		Block::Driver *create() { return driver; }
+		Block::Driver *create() override { return driver; }
 
-		void destroy(Block::Driver *driver) { }
+		void destroy(Block::Driver *driver) override
+		{
+			Genode::destroy(alloc, driver);
+			driver = nullptr;
+		}
 	};
 
 	Factory     factory { env, heap, announce_dispatcher };