[PATCH v2] scsi: cxgbi: fix retry_timer use-after-free on adapter detach

Fan Wu posted 1 patch 1 month, 2 weeks ago
drivers/scsi/cxgbi/cxgb3i/cxgb3i.c | 8 +++++++-
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 9 ++++++++-
drivers/scsi/cxgbi/libcxgbi.c      | 6 +++++-
3 files changed, 20 insertions(+), 3 deletions(-)
[PATCH v2] scsi: cxgbi: fix retry_timer use-after-free on adapter detach
Posted by Fan Wu 1 month, 2 weeks ago
An active-open connection may keep retry_timer armed because
cxgbi_sock_closed() returns early in CTP_ACTIVE_OPEN state.  On adapter
detach, the timer callback can then dereference csk->cdev after cdev has
been freed.

Set CTPF_OFFLOAD_DOWN under csk->lock during portmap cleanup, reject new
retry_timer arming, and make an already queued callback a no-op while
offload is down.  A connection still in CTP_ACTIVE_OPEN is pushed through
cxgbi_sock_fail_act_open() so that its ATID, L2T entry, port mapping, and
final reference are released while cdev is still valid, rather than left
for the now-disabled retry timer.  timer_delete_sync() then drains a
pending or running callback before the final cxgbi_sock_put().

This issue was found by an in-house static analysis tool.

Fixes: 7b36b6e03b0d ("[SCSI] cxgb4i v5: iscsi driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes in v2:
 - drop the lldi/t4 deref-move in csk_act_open_retry_timer(): csk->cdev is
   always valid when the callback runs (timer_delete_sync drains before the
   cdev free), per Varun Prakash.

 drivers/scsi/cxgbi/cxgb3i/cxgb3i.c | 8 +++++++-
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 9 ++++++++-
 drivers/scsi/cxgbi/libcxgbi.c      | 6 +++++-
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
index 69de965..f61f280 100644
--- 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;
+	}
 	skb = alloc_wr(sizeof(struct cpl_act_open_req), 0, GFP_ATOMIC);
 	if (!skb)
 		cxgbi_sock_fail_act_open(csk, -ENOMEM);
@@ -585,7 +590,8 @@ static int do_act_open_rpl(struct t3cdev *tdev, struct sk_buff *skb, void *ctx)
 
 	cxgbi_sock_get(csk);
 	spin_lock_bh(&csk->lock);
-	if (rpl->status == CPL_ERR_CONN_EXIST &&
+	if (!cxgbi_sock_flag(csk, CTPF_OFFLOAD_DOWN) &&
+	    rpl->status == CPL_ERR_CONN_EXIST &&
 	    csk->retry_timer.function != act_open_retry_timer) {
 		csk->retry_timer.function = act_open_retry_timer;
 		mod_timer(&csk->retry_timer, jiffies + HZ / 2);
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 4267662..7c9b465 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -1001,6 +1001,12 @@ static void csk_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;
+	}
+
 	if (t4) {
 		size = sizeof(struct cpl_act_open_req);
 		size6 = sizeof(struct cpl_act_open_req6);
@@ -1075,7 +1081,8 @@ static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
 	cxgbi_sock_get(csk);
 	spin_lock_bh(&csk->lock);
 
-	if (status == CPL_ERR_CONN_EXIST &&
+	if (!cxgbi_sock_flag(csk, CTPF_OFFLOAD_DOWN) &&
+	    status == CPL_ERR_CONN_EXIST &&
 	    csk->retry_timer.function != csk_act_open_retry_timer) {
 		csk->retry_timer.function = csk_act_open_retry_timer;
 		mod_timer(&csk->retry_timer, jiffies + HZ / 2);
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index ea9631b..131d4f0 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -107,8 +107,12 @@ void cxgbi_device_portmap_cleanup(struct cxgbi_device *cdev)
 				csk, cdev);
 			spin_lock_bh(&csk->lock);
 			cxgbi_sock_set_flag(csk, CTPF_OFFLOAD_DOWN);
-			cxgbi_sock_closed(csk);
+			if (csk->state == CTP_ACTIVE_OPEN)
+				cxgbi_sock_fail_act_open(csk, -ENODEV);
+			else
+				cxgbi_sock_closed(csk);
 			spin_unlock_bh(&csk->lock);
+			timer_delete_sync(&csk->retry_timer);
 			cxgbi_sock_put(csk);
 		}
 	}
-- 
2.34.1