Received-SPF: pass (google.com: domain of linux-kernel+bounces-5096-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <ac062d49-05bd-4615-ac38-4a463d533917@suse.de>
Date: Tue, 19 Dec 2023 12:41:50 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 14/16] nvmet-fc: free hostport after release reference
 to tgtport
Content-Language: en-US
To: Daniel Wagner <dwagner@suse.de>, linux-nvme@lists.infradead.org
Cc: linux-kernel@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
 Sagi Grimberg <sagi@grimberg.me>, Keith Busch <kbusch@kernel.org>,
 James Smart <james.smart@broadcom.com>
References: <20231218153105.12717-1-dwagner@suse.de>
 <20231218153105.12717-15-dwagner@suse.de>
From: Hannes Reinecke <hare@suse.de>
In-Reply-To: <20231218153105.12717-15-dwagner@suse.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 12/18/23 16:31, Daniel Wagner wrote:
> Give the ref back before destroying the hostport object to prevent a
> potential UAF.
> 
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>   drivers/nvme/target/fc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index 663c51c9fe53..23d8779dc221 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -986,8 +986,8 @@ nvmet_fc_hostport_free(struct kref *ref)
>   	spin_unlock_irqrestore(&tgtport->lock, flags);
>   	if (tgtport->ops->host_release && hostport->invalid)
>   		tgtport->ops->host_release(hostport->hosthandle);
> -	kfree(hostport);
>   	nvmet_fc_tgtport_put(tgtport);
> +	kfree(hostport);
>   }
>   
>   static void

That, I guess, needs some more explanation.
It's not immediately obvious why a 'put' on the targetport would
have implications on the hostport. But when it does wouldn't it
be more sensible to free up the reference to the hostport when the
final 'put' on the target port happens?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Ivo Totev, Andrew McDonald,
Werner Knoblich