Re: [PATCH v2 01/14] drbd: Open-code transfer log list walk

[sashiko-bot@kernel.org]

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 <chengkaitao@kylinos.cn>

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/20260609064122.95825-1-kaitao.cheng@linux.dev?part=1