Kernel: Fix race conditions processing async device requests

Kernel/Devices/AsyncDeviceRequest.cpp

@@ -91,15 +91,11 @@ void AsyncDeviceRequest::add_sub_request(NonnullRefPtr<AsyncDeviceRequest> sub_r
     VERIFY(sub_request->m_parent_request == nullptr);
     sub_request->m_parent_request = this;
 
-    bool should_start;
-    {
-        ScopedSpinLock lock(m_lock);
-        VERIFY(!is_completed_result(m_result));
-        m_sub_requests_pending.append(sub_request);
-        should_start = (m_result == Started);
-    }
-    if (should_start)
-        sub_request->do_start();
+    ScopedSpinLock lock(m_lock);
+    VERIFY(!is_completed_result(m_result));
+    m_sub_requests_pending.append(sub_request);
+    if (m_result == Started)
+        sub_request->do_start(move(lock));
 }
 
 void AsyncDeviceRequest::sub_request_finished(AsyncDeviceRequest& sub_request)

Kernel/Devices/AsyncDeviceRequest.h

@@ -78,9 +78,14 @@ public:
 
     [[nodiscard]] RequestWaitResult wait(Time* = nullptr);
 
-    void do_start(Badge<Device>)
+    void do_start(ScopedSpinLock<SpinLock<u8>>&& requests_lock)
     {
-        do_start();
+        if (is_completed_result(m_result))
+            return;
+        m_result = Started;
+        requests_lock.unlock();
+
+        start();
     }
 
     void complete(RequestResult result);
@@ -137,17 +142,6 @@ private:
     void sub_request_finished(AsyncDeviceRequest&);
     void request_finished();
 
-    void do_start()
-    {
-        {
-            ScopedSpinLock lock(m_lock);
-            if (is_completed_result(m_result))
-                return;
-            m_result = Started;
-        }
-        start();
-    }
-
     bool in_target_context(const UserOrKernelBuffer& buffer) const
     {
         if (buffer.is_kernel_buffer())

Kernel/Devices/Device.cpp

@@ -82,20 +82,15 @@ String Device::absolute_path(const FileDescription&) const
 
 void Device::process_next_queued_request(Badge<AsyncDeviceRequest>, const AsyncDeviceRequest& completed_request)
 {
-    AsyncDeviceRequest* next_request = nullptr;
-
-    {
-        ScopedSpinLock lock(m_requests_lock);
-        VERIFY(!m_requests.is_empty());
-        VERIFY(m_requests.first().ptr() == &completed_request);
-        m_requests.remove(m_requests.begin());
-        if (!m_requests.is_empty())
-            next_request = m_requests.first().ptr();
+    ScopedSpinLock lock(m_requests_lock);
+    VERIFY(!m_requests.is_empty());
+    VERIFY(m_requests.first().ptr() == &completed_request);
+    m_requests.remove(m_requests.begin());
+    if (!m_requests.is_empty()) {
+        auto* next_request = m_requests.first().ptr();
+        next_request->do_start(move(lock));
     }
 
-    if (next_request)
-        next_request->start();
-
     evaluate_block_conditions();
 }
 

Kernel/Devices/Device.h

@@ -72,14 +72,11 @@ public:
     NonnullRefPtr<AsyncRequestType> make_request(Args&&... args)
     {
         auto request = adopt(*new AsyncRequestType(*this, forward<Args>(args)...));
-        bool was_empty;
-        {
-            ScopedSpinLock lock(m_requests_lock);
-            was_empty = m_requests.is_empty();
-            m_requests.append(request);
-        }
+        ScopedSpinLock lock(m_requests_lock);
+        bool was_empty = m_requests.is_empty();
+        m_requests.append(request);
         if (was_empty)
-            request->do_start({});
+            request->do_start(move(lock));
         return request;
     }