linux-kernel - Re: [PATCH] nvme-fc: don't require user to enter host

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] [day] [month] [year] [list]

Message-ID: <ed7d5ef0-ef9f-3f3e-0ae5-9714ab83bf16@suse.de>
Date:   Thu, 30 Nov 2017 16:08:49 +0100
From:   Hannes Reinecke <hare@...e.de>
To:     Johannes Thumshirn <jthumshirn@...e.de>,
        Christoph Hellwig <hch@....de>
Cc:     Linux Kernel Mailinglist <linux-kernel@...r.kernel.org>,
        Sagi Grimberg <sagi@...mberg.me>,
        Keith Busch <keith.busch@...el.com>,
        Linux NVMe Mailinglist <linux-nvme@...ts.infradead.org>,
        James Smart <james.smart@...adcom.com>
Subject: Re: [PATCH] nvme-fc: don't require user to enter host_traddr

On 11/30/2017 03:56 PM, Johannes Thumshirn wrote:
> One major usability difference between NVMf RDMA and FC is resolving
> the default host transport address in RDMA. This is perfectly doable
> in FC as well, as we already have all possible lport <-> rport
> combinations pre-populated so we can pick the first lport that has a
> connection to our desired rport per default or optionally use the user
> supplied lport if we have one.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@...e.de>
> Cc: James Smart <james.smart@...adcom.com>
> ---
>  drivers/nvme/host/fc.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 7ab0be55c7d0..46ab900bbe26 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -3328,15 +3328,19 @@ nvme_fc_create_ctrl(struct device *dev, struct nvmf_ctrl_options *opts)
>  	if (ret || !raddr.nn || !raddr.pn)
>  		return ERR_PTR(-EINVAL);
>  
> -	ret = nvme_fc_parse_traddr(&laddr, opts->host_traddr, NVMF_TRADDR_SIZE);
> -	if (ret || !laddr.nn || !laddr.pn)
> -		return ERR_PTR(-EINVAL);
> +	if (opts->mask & NVMF_OPT_HOST_TRADDR) {
> +		ret = nvme_fc_parse_traddr(&laddr, opts->host_traddr,
> +					   NVMF_TRADDR_SIZE);
> +		if (ret || !laddr.nn || !laddr.pn)
> +			return ERR_PTR(-EINVAL);
> +	}
>  
>  	/* find the host and remote ports to connect together */
>  	spin_lock_irqsave(&nvme_fc_lock, flags);
>  	list_for_each_entry(lport, &nvme_fc_lport_list, port_list) {
> -		if (lport->localport.node_name != laddr.nn ||
> -		    lport->localport.port_name != laddr.pn)
> +		if ((laddr.nn || laddr.pn) &&
> +		    (lport->localport.node_name != laddr.nn ||
> +		     lport->localport.port_name != laddr.pn))
>  			continue;
>  
>  		list_for_each_entry(rport, &lport->endp_list, endp_list) {
We need both, 'laddr.nn' and 'laddr.pn'. So this statement is wrong.

You probably need something like

if (!laddr.nn || !laddr.pn || ....)

Cheers,

Hannes

-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@...e.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)