Re: [PATCH] scsi: libfc: fix directory server rport memory leak

[Hannes Reinecke <hare@suse.de>]

On 9/19/26 7:34 PM, Guangshuo Li wrote:
> fc_rport_recv_plogi_req() creates an rport before allocating the frame
> used for the PLOGI LS_ACC response.
> 
> fc_rport_create() does not add FC_FID_DIR_SERV rports to the discovery
> rport list. If fc_frame_alloc() fails while handling a PLOGI from the
> directory server, the function returns without starting the rport state
> machine or dropping the initial rport reference.
> 
> Since the directory server rport is not present in the discovery list,
> there is no later teardown path that can find the object and release
> that reference. The allocated fc_rport_priv is therefore leaked.
> 
> Record when the failed frame allocation leaves an unlisted directory
> server rport behind and drop its initial reference after releasing the
> rport mutex. Keep the existing lifetime unchanged for ordinary rports,
> which remain owned by the discovery list.
> 
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
> 
> Fixes: 3ac6f98f4113 ("[SCSI] libfc: correctly handle incoming PLOGI request.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>   drivers/scsi/libfc/fc_rport.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index c25979d96808..884b233c2f72 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -1848,6 +1848,7 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport,
>   	struct fc_els_flogi *pl;
>   	struct fc_seq_els_data rjt_data;
>   	u32 sid;
> +	bool drop_rdata = false;
>   
>   	lockdep_assert_held(&lport->lp_mutex);
>   
> @@ -1940,8 +1941,10 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport,
>   	 * Send LS_ACC.	 If this fails, the originator should retry.
>   	 */
>   	fp = fc_frame_alloc(lport, sizeof(*pl));
> -	if (!fp)
> +	if (!fp) {
> +		drop_rdata = sid == FC_FID_DIR_SERV;
>   		goto out;
> +	}
>   
>   	fc_plogi_fill(lport, fp, ELS_LS_ACC);
>   	fc_fill_reply_hdr(fp, rx_fp, FC_RCTL_ELS_REP, 0);
> @@ -1949,6 +1952,8 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport,
>   	fc_rport_enter_prli(rdata);
>   out:
>   	mutex_unlock(&rdata->rp_mutex);
> +	if (drop_rdata)
> +		kref_put(&rdata->kref, fc_rport_destroy);
>   	fc_frame_free(rx_fp);
>   	return;
>   

Wouldn't it be better to always create an rport for the directory
server?
That would make the teardown path far easier.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich