[PATCH] firewire: core: consolidate lock scopes for event waiting in dequeue function

[Takashi Sakamoto]

Current implementation uses the spinlock in the card structure to
serialize accesses to the event list. The dequeue_event() function yields
the CPU while waiting for the condition to become true, but checks the
list without acquiring the spinlock. It acquires the spinlock when
operating the list, however it assumes that the list has at least one
entry.

This can cause problems when multiple threads execute read(2) system calls
concurrently, since the list can become empty after rescheduling but before
the list operation, depending on the scheduling order of the two threads.

Use a wait_event variant that reacquires the lock after being
rescheduled, before checking the condition.

Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
---
 drivers/firewire/core-cdev.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index e49d8a58be09..54f7376ef1f5 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -356,19 +356,27 @@ static int dequeue_event(struct client *client,
 	size_t size, total;
 	int i, ret;
 
-	ret = wait_event_interruptible(client->wait,
-			!list_empty(&client->event_list) ||
-			fw_device_is_shutdown(client->device));
-	if (ret < 0)
-		return ret;
+	// After the following block, the event pointer above is guaranteed to have a correct value.
+	{
+		spin_lock_irq(&client->lock);
 
-	if (list_empty(&client->event_list) &&
-		       fw_device_is_shutdown(client->device))
-		return -ENODEV;
+		int ret = wait_event_interruptible_lock_irq(client->wait,
+			!list_empty(&client->event_list) || fw_device_is_shutdown(client->device),
+			client->lock);
+		if (ret < 0) {
+			spin_unlock_irq(&client->lock);
+			return ret;
+		}
+
+		if (fw_device_is_shutdown(client->device)) {
+			spin_unlock_irq(&client->lock);
+			return -ENODEV;
+		}
 
-	scoped_guard(spinlock_irq, &client->lock) {
 		event = list_first_entry(&client->event_list, struct event, link);
 		list_del(&event->link);
+
+		spin_unlock_irq(&client->lock);
 	}
 
 	total = 0;
-- 
2.53.0