linux-kernel - Re: [PATCH v5 07/14] nvmet-fcloop: access fcpreq only when holding reqlock

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: <c86e1da9-0525-4b4a-a32e-03d60f78d8fb@flourine.local>
Date: Thu, 24 Apr 2025 13:31:13 +0200
From: Daniel Wagner <dwagner@...e.de>
To: Hannes Reinecke <hare@...e.de>
Cc: 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>, Keith Busch <kbusch@...nel.org>, linux-nvme@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 07/14] nvmet-fcloop: access fcpreq only when holding
 reqlock

On Thu, Apr 24, 2025 at 12:15:20PM +0200, Hannes Reinecke wrote:
> > @@ -663,9 +667,10 @@ fcloop_fcp_abort_recv_work(struct work_struct *work)
> >   	unsigned long flags;
> >   	spin_lock_irqsave(&tfcp_req->reqlock, flags);
> > -	fcpreq = tfcp_req->fcpreq;
> >   	switch (tfcp_req->inistate) {
> >   	case INI_IO_ABORTED:
> > +		fcpreq = tfcp_req->fcpreq;
> > +		tfcp_req->fcpreq = NULL;
> >   		break;
> >   	case INI_IO_COMPLETED:
> >   		completed = true;
> > @@ -688,10 +693,6 @@ fcloop_fcp_abort_recv_work(struct work_struct *work)
> >   		nvmet_fc_rcv_fcp_abort(tfcp_req->tport->targetport,
> >   					&tfcp_req->tgt_fcp_req);
> > -	spin_lock_irqsave(&tfcp_req->reqlock, flags);
> > -	tfcp_req->fcpreq = NULL;
> > -	spin_unlock_irqrestore(&tfcp_req->reqlock, flags);
> > -
> What happens for INI_IO_COMPLETED?

The request was completed before the abort handler was running. Thus
nothing to do here.

> Don't we need to clear the 'fcpreq' pointer in that case, too?

The normal code path has already taken care of this request and the only
thing left to do is to give back the refcount on the tfcp_req to free
the memory.