Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Dropping `req_lock` during transfer log iteration allows the current request to be moved to a different list, causing out-of-bounds reads and an infinite loop. -- commit eaa3f881c5752fd80fb9f58149972d66629f2d5b Author: Kaitao Cheng <[email protected]> drbd: Open-code transfer log list walk This commit open-codes the transfer log list walk in drbd_debugfs.c to preserve the existing traversal semantics where the cursor is revalidated after temporarily dropping a lock, in preparation for an upcoming change to list_for_each_entry(). > diff --git a/drivers/block/drbd/drbd_debugfs.c > b/drivers/block/drbd/drbd_debugfs.c > index 12460b584bcb9..e90cead90e9db 100644 > --- a/drivers/block/drbd/drbd_debugfs.c > +++ b/drivers/block/drbd/drbd_debugfs.c > @@ -308,7 +308,9 @@ static void > seq_print_resource_transfer_log_summary(struct seq_file *m, > > seq_puts(m, "n\tdevice\tvnr\t" RQ_HDR); > spin_lock_irq(&resource->req_lock); > - list_for_each_entry(req, &connection->transfer_log, tl_requests) { > + for (req = list_first_entry(&connection->transfer_log, typeof(*req), > tl_requests); > + !list_entry_is_head(req, &connection->transfer_log, tl_requests); > + req = list_next_entry(req, tl_requests)) { [Severity: High] This is a pre-existing issue, but can this loop run infinitely or read out-of-bounds memory when resource->req_lock is dropped inside the loop body? Later in seq_print_resource_transfer_log_summary(), the lock is dropped to yield: spin_unlock_irq(&resource->req_lock); cond_resched(); spin_lock_irq(&resource->req_lock); req_next = list_next_entry(req, tl_requests); While the lock is dropped, if another thread calls drbd_restart_request() in drivers/block/drbd/drbd_main.c, the current req can be moved from connection->transfer_log to the global retry.writes list via list_move_tail(): drivers/block/drbd/drbd_main.c:drbd_restart_request() { ... spin_lock_irqsave(&retry.lock, flags); list_move_tail(&req->tl_requests, &retry.writes); spin_unlock_irqrestore(&retry.lock, flags); ... } When the lock is reacquired, req remains valid because of the kref_get() call, but list_next_entry() will now follow retry.writes instead of transfer_log. Because the loop terminates by checking against &connection->transfer_log, traversing retry.writes bypasses the break condition. Furthermore, if do_retry() concurrently splices retry.writes onto a local stack list, can this traversal continue into unmapped or overwritten stack memory? > unsigned int tmp = 0; > unsigned int s; > ++count; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
