netdev - Re: [PATCH net] net/smc: fix incorrect SMC-D link group matching logic

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: <54f9622f-7e62-4089-91dc-df541f8f37f8@linux.ibm.com>
Date: Thu, 25 Jan 2024 16:26:22 +0100
From: Alexandra Winter <wintera@...ux.ibm.com>
To: Wen Gu <guwen@...ux.alibaba.com>, mjrosato@...ux.ibm.com,
        wenjia@...ux.ibm.com, jaka@...ux.ibm.com, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Cc: alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
        linux-s390@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net/smc: fix incorrect SMC-D link group matching
 logic



On 25.01.24 13:39, Wen Gu wrote:
> The logic to determine if SMC-D link group matches is incorrect. The
> correct logic should be that it only returns true when the GID is the
> same, and the SMC-D device is the same and the extended GID is the same
> (in the case of virtual ISM).
> 
> It can be fixed by adding brackets around the conditional (or ternary)
> operator expression. But for better readability and maintainability, it
> has been changed to an if-else statement.
> 
> Reported-by: Matthew Rosato <mjrosato@...ux.ibm.com>
> Closes: https://lore.kernel.org/r/13579588-eb9d-4626-a063-c0b77ed80f11@linux.ibm.com
> Fixes: b40584d14570 ("net/smc: compatible with 128-bits extended GID of virtual ISM device")
> Link: https://lore.kernel.org/r/13579588-eb9d-4626-a063-c0b77ed80f11@linux.ibm.com
> Signed-off-by: Wen Gu <guwen@...ux.alibaba.com>
> ---
>  net/smc/smc_core.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 95cc95458e2d..e4c858411207 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1877,9 +1877,15 @@ static bool smcd_lgr_match(struct smc_link_group *lgr,
>  			   struct smcd_dev *smcismdev,
>  			   struct smcd_gid *peer_gid)
>  {
> -	return lgr->peer_gid.gid == peer_gid->gid && lgr->smcd == smcismdev &&
> -		smc_ism_is_virtual(smcismdev) ?
> -		(lgr->peer_gid.gid_ext == peer_gid->gid_ext) : 1;
> +	if (lgr->peer_gid.gid != peer_gid->gid ||
> +	    lgr->smcd != smcismdev)
> +		return false;
> +
> +	if (smc_ism_is_virtual(smcismdev) &&
> +	    lgr->peer_gid.gid_ext != peer_gid->gid_ext)
> +		return false;
> +
> +	return true;
>  }
>  
>  /* create a new SMC connection (and a new link group if necessary) */

Thank you Wen Gu

Reviewed-by: Alexandra Winter <wintera@...ux.ibm.com>