Re: [PATCH] scsi: libfc: Fix rport kref leak in fc_rport_enter_ready()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] scsi: libfc: reference leak in fc_rport_work() [1]
--

commit f18e1c7bd3d9738ea4413b32cfed69e7594a5a24
Author: Weidong Wang <weidongw@illinois.edu>

scsi: libfc: Fix rport kref leak in fc_rport_enter_ready()

This commit avoids taking a reference on rdata->kref in
fc_rport_enter_ready() when rdata->event is already pending and
rdata->event_work will not be queued.

> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index c25979d96808a..7b093854462e4 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -550,10 +550,11 @@ static void fc_rport_enter_ready(struct fc_rport_priv *rdata)
>  
>  	FC_RPORT_DBG(rdata, "Port is Ready\n");
>  
> -	kref_get(&rdata->kref);
> -	if (rdata->event == RPORT_EV_NONE &&
> -	    !queue_work(rport_event_queue, &rdata->event_work))
> -		kref_put(&rdata->kref, fc_rport_destroy);
> +	if (rdata->event == RPORT_EV_NONE) {
> +		kref_get(&rdata->kref);
> +		if (!queue_work(rport_event_queue, &rdata->event_work))
> +			kref_put(&rdata->kref, fc_rport_destroy);
> +	}
>  
>  	rdata->event = RPORT_EV_READY;
>  }

[Severity: Medium]
This problem wasn't introduced by this patch, but doesn't the RPORT_EV_READY
handler in fc_rport_work() also leak the workqueue reference taken here when
fc_remote_port_add() fails [1]?

drivers/scsi/libfc/fc_rport.c:fc_rport_work() {
	switch (event) {
	case RPORT_EV_READY:
		...
		kref_get(&rdata->kref);
		mutex_unlock(&rdata->rp_mutex);

		if (!rport) {
			FC_RPORT_DBG(rdata, "No rport!\n");
			rport = fc_remote_port_add(lport->host, 0, &ids);
		}
		if (!rport) {
			FC_RPORT_DBG(rdata, "Failed to add the rport\n");
			fc_rport_logoff(rdata);
			kref_put(&rdata->kref, fc_rport_destroy);
			return;
		}
		...
		kref_put(&rdata->kref, fc_rport_destroy);
		break;
	...
	}
	kref_put(&rdata->kref, fc_rport_destroy);
}

The error path drops the local reference taken before unlocking rp_mutex,
and then returns directly instead of breaking out to the final kref_put() at
the end of fc_rport_work(), permanently leaking rdata.

[1] https://sashiko.dev/#/bug/linux-f352567a-e977-44cd-9965-6ccd520fb22b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108-h123-send-v1-1-b7672ff849f0@illinois.edu?part=1