netdev - Re: [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for SMC-R

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: <9a7c5736-1f85-5743-8963-ce8a2d27f83b@linux.ibm.com>
Date:   Tue, 25 Feb 2020 16:17:16 +0100
From:   Karsten Graul <kgraul@...ux.ibm.com>
To:     Hans Wippel <ndev@...pl.net>, ubraun@...ux.ibm.com,
        davem@...emloft.net
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/2] net/smc: improve peer ID in CLC decline for
 SMC-R



On 25/02/2020 00:59, Hans Wippel wrote:
> According to RFC 7609, all CLC messages contain a peer ID that consists
> of a unique instance ID and the MAC address of one of the host's RoCE
> devices. But if a SMC-R connection cannot be established, e.g., because
> no matching pnet table entry is found, the current implementation uses a
> zero value in the CLC decline message although the host's peer ID is set
> to a proper value.
> 
> If no RoCE and no ISM device is usable for a connection, there is no LGR
> and the LGR check in smc_clc_send_decline() prevents that the peer ID is
> copied into the CLC decline message for both SMC-D and SMC-R. So, this
> patch modifies the check to also accept the case of no LGR. Also, only a
> valid peer ID is copied into the decline message.
> 
> Signed-off-by: Hans Wippel <ndev@...pl.net>
> ---
>  net/smc/smc_clc.c | 9 ++++++---
>  net/smc/smc_ib.h  | 1 +
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 3e16b887cfcf..e2d3b5b95632 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -372,9 +372,12 @@ int smc_clc_send_decline(struct smc_sock *smc, u32 peer_diag_info)
>  	dclc.hdr.length = htons(sizeof(struct smc_clc_msg_decline));
>  	dclc.hdr.version = SMC_CLC_V1;
>  	dclc.hdr.flag = (peer_diag_info == SMC_CLC_DECL_SYNCERR) ? 1 : 0;
> -	if (smc->conn.lgr && !smc->conn.lgr->is_smcd)
> -		memcpy(dclc.id_for_peer, local_systemid,
> -		       sizeof(local_systemid));
> +	if (!smc->conn.lgr || !smc->conn.lgr->is_smcd) {
> +		if (smc_ib_is_valid_local_systemid()) {
> +			memcpy(dclc.id_for_peer, local_systemid,
> +			       sizeof(local_systemid));
> +		}
                ^
Adds curly braces for no reason.

> +	}
>  	dclc.peer_diagnosis = htonl(peer_diag_info);
>  	memcpy(dclc.trl.eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));
>  
> diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
> index 255db87547d3..5c2b115d36da 100644
> --- a/net/smc/smc_ib.h
> +++ b/net/smc/smc_ib.h
> @@ -84,4 +84,5 @@ void smc_ib_sync_sg_for_device(struct smc_ib_device *smcibdev,
>  			       enum dma_data_direction data_direction);
>  int smc_ib_determine_gid(struct smc_ib_device *smcibdev, u8 ibport,
>  			 unsigned short vlan_id, u8 gid[], u8 *sgid_index);
> +bool smc_ib_is_valid_local_systemid(void);
>  #endif
> 

-- 
Karsten

(I'm a dude)