linux-kernel - Re: [PATCH v3 10/18] nvmet-fcloop: allocate/free fcloop

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: <603b438e-02f6-4f01-8ffa-12ab6ec8e745@suse.de>
Date: Tue, 18 Mar 2025 12:17:11 +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 10/18] nvmet-fcloop: allocate/free fcloop_lsreq
 directly

On 3/18/25 11:40, Daniel Wagner wrote:
> fcloop depends on the host or the target to allocate the fcloop_lsreq
> object. This means that the lifetime of the fcloop_lsreq is tied to
> either the host or the target. Consequently, the host or the target must
> cooperate during shutdown.
> 
> Unfortunately, this approach does not work well when the target forces a
> shutdown, as there are dependencies that are difficult to resolve in a
> clean way.
> 
> The simplest solution is to decouple the lifetime of the fcloop_lsreq
> object by managing them directly within fcloop. Since this is not a
> performance-critical path and only a small number of LS objects are used
> during setup and cleanup, it does not significantly impact performance
> to allocate them during normal operation.
> 
> Signed-off-by: Daniel Wagner <wagi@...nel.org>
> ---
>   drivers/nvme/target/fcloop.c | 53 +++++++++++++++++++++++++++++---------------
>   1 file changed, 35 insertions(+), 18 deletions(-)
> 
 > diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target 
fcloop.c> index 
06f42da6a0335c53ae319133119d057aab12e07e..537fc6533a4cf5d39855cf850b82af739eeb3056 
100644
> --- a/drivers/nvme/target/fcloop.c
> +++ b/drivers/nvme/target/fcloop.c
> @@ -342,6 +342,7 @@ fcloop_rport_lsrqst_work(struct work_struct *work)
>   		 * callee may free memory containing tls_req.
>   		 * do not reference lsreq after this.
>   		 */
> +		kfree(tls_req);
>   
>   		spin_lock(&rport->lock);
>   	}
> @@ -353,10 +354,13 @@ fcloop_h2t_ls_req(struct nvme_fc_local_port *localport,
>   			struct nvme_fc_remote_port *remoteport,
>   			struct nvmefc_ls_req *lsreq)
>   {
> -	struct fcloop_lsreq *tls_req = lsreq->private;
>   	struct fcloop_rport *rport = remoteport->private;
> +	struct fcloop_lsreq *tls_req;
>   	int ret = 0;
>   
> +	tls_req = kmalloc(sizeof(*tls_req), GFP_KERNEL);
> +	if (!tls_req)
> +		return -ENOMEM;

This cries out for kmem_cache_alloc() ...

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