From: sashiko-bot@kernel.org
To: "Fan Wu" <fanwu01@zju.edu.cn>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] scsi: cxgbi: fix retry_timer use-after-free on adapter detach
Date: Wed, 05 Aug 2026 01:52:23 +0000 [thread overview]
Message-ID: <20260805015224.BE3BD1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260805013209.418861-1-fanwu01@zju.edu.cn>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Sockets in CTP_ACTIVE_OPEN state permanently leak ATID resources and memory on adapter detach.
- [High] Non-synchronous timer_delete() followed by socket destruction leads to a Use-After-Free.
- [High] Missing state check in the retry timer callbacks sends duplicate open requests, destroying established connections.
--
--- Patch [1]: [PATCH] scsi: cxgbi: fix retry_timer use-after-free on adapter detach ---
commit a8e38255da09728f75742d7fed123ba4827cbc81
Author: Fan Wu <fanwu01@zju.edu.cn>
scsi: cxgbi: fix retry_timer use-after-free on adapter detach
This commit fixes a use-after-free on adapter detach by setting the
CTPF_OFFLOAD_DOWN flag under the socket lock and using timer_delete_sync()
to drain pending callbacks. It also updates the timer callbacks and reply
handlers to check this flag before proceeding.
> diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
> --- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
> +++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
> @@ -556,6 +556,11 @@ static void act_open_retry_timer(struct timer_list *t)
>
> cxgbi_sock_get(csk);
> spin_lock_bh(&csk->lock);
> + if (cxgbi_sock_flag(csk, CTPF_OFFLOAD_DOWN)) {
> + spin_unlock_bh(&csk->lock);
> + cxgbi_sock_put(csk);
> + return;
> + }
[Severity: High]
This is a pre-existing issue, but does this callback send duplicate open
requests that can destroy established connections?
The callback unconditionally sends an open request without verifying if
csk->state is still CTP_ACTIVE_OPEN. If the timer callback executes
immediately after do_act_establish() transitions the socket to
CTP_ESTABLISHED and drops the lock, the callback will acquire the lock
and blindly send a duplicate CPL_ACT_OPEN_REQ.
The hardware will reject this with CPL_ERR_CONN_EXIST, which causes
do_act_open_rpl() to forcibly tear down the valid connection.
[Severity: High]
This is also a pre-existing issue, but can the non-synchronous timer
cancellation in do_act_establish() lead to a use-after-free?
In do_act_establish(), the timer is canceled non-synchronously:
do_act_establish() {
...
if (csk->retry_timer.function) {
timer_delete(&csk->retry_timer);
csk->retry_timer.function = NULL;
}
...
}
If the timer callback is queued on another CPU, it can execute after
the socket drops its lock. If the new connection is rapidly aborted
(e.g., by receiving CPL_ABORT_REQ_RSS), the socket teardown path releases
all references and frees the socket memory.
When the delayed timer callback eventually runs, it will call
cxgbi_sock_get(csk) on freed memory.
[ ... ]
> diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
> --- a/drivers/scsi/cxgbi/libcxgbi.c
> +++ b/drivers/scsi/cxgbi/libcxgbi.c
> @@ -109,6 +109,7 @@ void cxgbi_device_portmap_cleanup(struct cxgbi_device *cdev)
> cxgbi_sock_set_flag(csk, CTPF_OFFLOAD_DOWN);
> cxgbi_sock_closed(csk);
> spin_unlock_bh(&csk->lock);
> + timer_delete_sync(&csk->retry_timer);
> cxgbi_sock_put(csk);
[Severity: High]
This is a pre-existing issue, but does this leak ATID resources and socket
memory for sockets in the CTP_ACTIVE_OPEN state?
Looking at cxgbi_sock_closed(), it returns early for ACTIVE_OPEN sockets:
cxgbi_sock_closed() {
...
if (csk->state == CTP_ACTIVE_OPEN || csk->state == CTP_CLOSED)
return;
}
This early return bypasses csk_release_offload_resources(). If an adapter
is detached while an active-open connection is pending, the timer callbacks
now immediately return upon seeing CTPF_OFFLOAD_DOWN without advancing
the state or failing the connection.
Without further hardware replies to fail the connection, the socket remains
indefinitely trapped in CTP_ACTIVE_OPEN, leaking the ATID and socket memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805013209.418861-1-fanwu01@zju.edu.cn?part=1
prev parent reply other threads:[~2026-08-05 1:52 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-05 1:32 [PATCH] scsi: cxgbi: fix retry_timer use-after-free on adapter detach Fan Wu
2026-08-05 1:52 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260805015224.BE3BD1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=fanwu01@zju.edu.cn \
--cc=linux-scsi@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.