linux-kernel - Re: [PATCH v3 16/18] nvmet-fc: take tgtport refs for portentry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f0e47ac3-d736-458d-8ffc-f7648174aa93@suse.de>
Date: Tue, 18 Mar 2025 12:19:18 +0100
From: Hannes Reinecke <hare@...e.de>
To: Daniel Wagner <wagi@...nel.org>, James Smart <james.smart@...adcom.com>,
 Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
 Chaitanya Kulkarni <kch@...dia.com>
Cc: Keith Busch <kbusch@...nel.org>, linux-nvme@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 16/18] nvmet-fc: take tgtport refs for portentry

On 3/18/25 11:40, Daniel Wagner wrote:
> Ensure that the tgtport is not going away as long portentry has a
> pointer on it.
> 
> Signed-off-by: Daniel Wagner <wagi@...nel.org>
> ---
>   drivers/nvme/target/fc.c | 45 +++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index d10ddcb57c1b09d871152f0d9a48f93ec6dc8685..649afce908bbade0a843efc4b8b76105c164b035 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -1265,6 +1265,7 @@ nvmet_fc_portentry_bind(struct nvmet_fc_tgtport *tgtport,
>   {
>   	lockdep_assert_held(&nvmet_fc_tgtlock);
>   
> +	nvmet_fc_tgtport_get(tgtport);
>   	pe->tgtport = tgtport;
>   	tgtport->pe = pe;
>   
> @@ -1284,8 +1285,10 @@ nvmet_fc_portentry_unbind(struct nvmet_fc_port_entry *pe)
>   	unsigned long flags;
>   
>   	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
> -	if (pe->tgtport)
> +	if (pe->tgtport) {
> +		nvmet_fc_tgtport_put(pe->tgtport);
>   		pe->tgtport->pe = NULL;
> +	}
>   	list_del(&pe->pe_list);
>   	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
>   }
> @@ -1303,8 +1306,10 @@ nvmet_fc_portentry_unbind_tgt(struct nvmet_fc_tgtport *tgtport)
>   
>   	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
>   	pe = tgtport->pe;
> -	if (pe)
> +	if (pe) {
> +		nvmet_fc_tgtport_put(pe->tgtport);
>   		pe->tgtport = NULL;
> +	}
>   	tgtport->pe = NULL;
>   	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
>   }
> @@ -1327,6 +1332,9 @@ nvmet_fc_portentry_rebind_tgt(struct nvmet_fc_tgtport *tgtport)
>   	list_for_each_entry(pe, &nvmet_fc_portentry_list, pe_list) {
>   		if (tgtport->fc_target_port.node_name == pe->node_name &&
>   		    tgtport->fc_target_port.port_name == pe->port_name) {
> +			if (!nvmet_fc_tgtport_get(tgtport))
> +				continue;
> +
>   			WARN_ON(pe->tgtport);
>   			tgtport->pe = pe;
>   			pe->tgtport = tgtport;
> @@ -1664,7 +1672,6 @@ nvmet_fc_unregister_targetport(struct nvmet_fc_target_port *target_port)
>   }
>   EXPORT_SYMBOL_GPL(nvmet_fc_unregister_targetport);
>   
> -
>   /* ********************** FC-NVME LS RCV Handling ************************* */
>   
>   

Empty line ...

> @@ -2901,12 +2908,17 @@ nvmet_fc_add_port(struct nvmet_port *port)
>   	list_for_each_entry(tgtport, &nvmet_fc_target_list, tgt_list) {
>   		if ((tgtport->fc_target_port.node_name == traddr.nn) &&
>   		    (tgtport->fc_target_port.port_name == traddr.pn)) {
> +			if (!nvmet_fc_tgtport_get(tgtport))
> +				continue;
> +
>   			/* a FC port can only be 1 nvmet port id */
>   			if (!tgtport->pe) {
>   				nvmet_fc_portentry_bind(tgtport, pe, port);
>   				ret = 0;
>   			} else
>   				ret = -EALREADY;
> +
> +			nvmet_fc_tgtport_put(tgtport);
>   			break;
>   		}
>   	}
> @@ -2922,11 +2934,21 @@ static void
>   nvmet_fc_remove_port(struct nvmet_port *port)
>   {
>   	struct nvmet_fc_port_entry *pe = port->priv;
> +	struct nvmet_fc_tgtport *tgtport = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
> +	if (pe->tgtport && nvmet_fc_tgtport_get(pe->tgtport))
> +		tgtport = pe->tgtport;
> +	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
>   
>   	nvmet_fc_portentry_unbind(pe);
>   
> -	/* terminate any outstanding associations */
> -	__nvmet_fc_free_assocs(pe->tgtport);
> +	if (tgtport) {
> +		/* terminate any outstanding associations */
> +		__nvmet_fc_free_assocs(tgtport);
> +		nvmet_fc_tgtport_put(tgtport);
> +	}
>   
>   	kfree(pe);
>   }
> @@ -2935,10 +2957,21 @@ static void
>   nvmet_fc_discovery_chg(struct nvmet_port *port)
>   {
>   	struct nvmet_fc_port_entry *pe = port->priv;
> -	struct nvmet_fc_tgtport *tgtport = pe->tgtport;
> +	struct nvmet_fc_tgtport *tgtport = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
> +	if (pe->tgtport && nvmet_fc_tgtport_get(pe->tgtport))
> +		tgtport = pe->tgtport;
> +	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
> +
> +	if (!tgtport)
> +		return;
>   
>   	if (tgtport && tgtport->ops->discovery_event)
>   		tgtport->ops->discovery_event(&tgtport->fc_target_port);
> +
> +	nvmet_fc_tgtport_put(tgtport);
>   }
>   
>   static ssize_t
> 
Otherwise:

Reviewed-by: Hannes Reinecke <hare@...e.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@...e.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