Re: [PATCH] scsi: target: cxgbit: fix refcount leak in __cxgbit_free_cdev_np

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] The patch introduces critical use-after-free and double-free vulnerabilities by unconditionally dropping the `cnp` reference count on both the success and timeout paths, violating the driver's asynchronous reply handling model.

Pre-existing issues:
- [Critical] When `__cxgbit_free_cdev_np()` times out, it aborts cleanup and causes the caller `cxgbit_free_all_np()` to break its loop, leaving dangling STID references to `cnp` on remaining devices.
--

commit f433423e199b1a3438bcf4e0bc324b24ab3b6629
Author: WenTao Liang <vulab@iscas.ac.cn>

scsi: target: cxgbit: fix refcount leak in __cxgbit_free_cdev_np

__cxgbit_free_cdev_np() calls cxgbit_get_cnp() to take a reference
on cnp for the duration of the firmware remove-server operation.
This patch attempts to release the reference on the timeout and
success paths.

Cc: stable@vger.kernel.org
Fixes: 9730ffcb8957 ("cxgbit: add files for cxgbit.ko")

> diff --git a/drivers/target/iscsi/cxgbit/cxgbit_cm.c b/drivers/target/iscsi/cxgbit/cxgbit_cm.c
> index 146705845fa3e..f8cd6457666b0 100644
> --- a/drivers/target/iscsi/cxgbit/cxgbit_cm.c
> +++ b/drivers/target/iscsi/cxgbit/cxgbit_cm.c
> @@ -544,8 +544,10 @@ __cxgbit_free_cdev_np(struct cxgbit_device *cdev, struct cxgbit_np *cnp)
>  
>  	ret = cxgbit_wait_for_reply(cdev, &cnp->com.wr_wait,
>  				    0, 10, __func__);
> -	if (ret == -ETIMEDOUT)
> +	if (ret == -ETIMEDOUT) {
> +		cxgbit_put_cnp(cnp);
>  		return ret;
> +	}

[Severity: Critical]
Does this cause a use-after-free and double-free if the firmware
reply is delayed?

In the Chelsio hardware transaction model, the reference acquired by
cxgbit_get_cnp() before sending the remove-server command is strictly
intended to be consumed by the asynchronous firmware reply handler,
cxgbit_close_listsrv_rpl().

If we time out and drop the reference here, the firmware request is
still pending and the STID is not unlinked. When the delayed reply
eventually arrives, cxgbit_close_listsrv_rpl() will look up the STID,
retrieve the already-freed cnp object, dereference it to wake up the
wait queue, and attempt to drop the reference again:

drivers/target/iscsi/cxgbit/cxgbit_cm.c:cxgbit_close_listsrv_rpl() {
    ...
    cxgbit_wake_up(&cnp->com.wr_wait, __func__, rpl->status);
    cxgbit_put_cnp(cnp);
    ...
}

[Severity: Critical]
This is a pre-existing issue, but does this timeout path leave
dangling STID references that lead to a use-after-free on incoming
connections?

If __cxgbit_free_cdev_np() returns -ETIMEDOUT, the caller
cxgbit_free_all_np() will break its loop and abandon the removal
of STIDs on subsequent healthy devices:

drivers/target/iscsi/cxgbit/cxgbit_cm.c:cxgbit_free_all_np() {
    ...
    ret = __cxgbit_free_cdev_np(cdev, cnp);
    if (ret == -ETIMEDOUT)
        break;
    ...
}

The parent caller cxgbit_free_np() then drops its base reference, freeing
the cnp object. Because the remaining devices still have active STID
registrations pointing to this freed cnp, any incoming connection request
handled by cxgbit_pass_accept_req() will look up the dangling STID and
access the freed cnp.

>  
>  	if (ipv6 && cnp->com.cdev) {
>  		struct sockaddr_in6 *sin6;
[ ... ]
> @@ -558,6 +560,7 @@ __cxgbit_free_cdev_np(struct cxgbit_device *cdev, struct cxgbit_np *cnp)
>  
>  	cxgb4_free_stid(cdev->lldi.tids, stid,
>  			cnp->com.local_addr.ss_family);
> +	cxgbit_put_cnp(cnp);

[Severity: Critical]
Does this addition cause an immediate double-free on the success path?

On the success path where cxgbit_wait_for_reply() returns 0, the
asynchronous reply handler cxgbit_close_listsrv_rpl() has already
executed and dropped the reference originally acquired by
cxgbit_get_cnp(). Adding an unconditional cxgbit_put_cnp(cnp) here
will attempt to drop a reference that has already been consumed.

>  	return 0;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611122203.78379-1-vulab@iscas.ac.cn?part=1