From e64f2d4de1f344b84c0fa0bebf3102a8fac48b33 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sun, 20 Sep 2026 19:47:45 +0900 Subject: [PATCH] firewire: core: consolidate lock scopes for event waiting in dequeue function 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. Link: https://lore.kernel.org/r/20260920104745.212753-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto --- 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 398bebaad064..1b5d75f7e075 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;