Skip to content

Commit 20e01bb

Browse files
committed
firewire: core: fix race condition against transaction list
The list of transaction is enumerated without acquiring card lock when processing AR response event. This causes a race condition bug when processing AT request completion event concurrently. This commit fixes the bug by put timer start for split transaction expiration into the scope of lock. The value of jiffies in card structure is referred before acquiring the lock. Cc: stable@vger.kernel.org # v6.18 Fixes: b5725cf ("firewire: core: use spin lock specific to timer for split transaction") Reported-by: Andreas Persson <andreasp56@outlook.com> Closes: alsa-project/snd-firewire-ctl-services#209 Tested-by: Andreas Persson <andreasp56@outlook.com> Link: https://lore.kernel.org/r/20260127223413.22265-1-o-takashi@sakamocchi.jp Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
1 parent 63804fe commit 20e01bb

1 file changed

Lines changed: 10 additions & 9 deletions

File tree

drivers/firewire/core-transaction.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -173,20 +173,14 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
173173
}
174174
}
175175

176-
static void start_split_transaction_timeout(struct fw_transaction *t,
177-
struct fw_card *card)
176+
// card->transactions.lock should be acquired in advance for the linked list.
177+
static void start_split_transaction_timeout(struct fw_transaction *t, unsigned int delta)
178178
{
179-
unsigned long delta;
180-
181179
if (list_empty(&t->link) || WARN_ON(t->is_split_transaction))
182180
return;
183181

184182
t->is_split_transaction = true;
185183

186-
// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
187-
// local destination never runs in any type of IRQ context.
188-
scoped_guard(spinlock_irqsave, &card->split_timeout.lock)
189-
delta = card->split_timeout.jiffies;
190184
mod_timer(&t->split_timeout_timer, jiffies + delta);
191185
}
192186

@@ -207,13 +201,20 @@ static void transmit_complete_callback(struct fw_packet *packet,
207201
break;
208202
case ACK_PENDING:
209203
{
204+
unsigned int delta;
205+
210206
// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
211207
// local destination never runs in any type of IRQ context.
212208
scoped_guard(spinlock_irqsave, &card->split_timeout.lock) {
213209
t->split_timeout_cycle =
214210
compute_split_timeout_timestamp(card, packet->timestamp) & 0xffff;
211+
delta = card->split_timeout.jiffies;
215212
}
216-
start_split_transaction_timeout(t, card);
213+
214+
// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
215+
// local destination never runs in any type of IRQ context.
216+
scoped_guard(spinlock_irqsave, &card->transactions.lock)
217+
start_split_transaction_timeout(t, delta);
217218
break;
218219
}
219220
case ACK_BUSY_X:

0 commit comments

Comments
 (0)