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: <8090bb34-1b70-43ea-ae13-df5d9a5eb761@linux.ibm.com>
Date: Thu, 25 Jan 2024 09:26:59 +0100
From: Alexandra Winter <wintera@...ux.ibm.com>
To: Wen Gu <guwen@...ux.alibaba.com>, wenjia@...ux.ibm.com, hca@...ux.ibm.com,
        gor@...ux.ibm.com, agordeev@...ux.ibm.com, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        jaka@...ux.ibm.com, Matthew Rosato <mjrosato@...ux.ibm.com>
Cc: Linux regressions mailing list <regressions@...ts.linux.dev>,
        borntraeger@...ux.ibm.com, svens@...ux.ibm.com,
        alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
        raspl@...ux.ibm.com, schnelle@...ux.ibm.com,
        guangguan.wang@...ux.alibaba.com, linux-s390@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: [REGRESSION] v6.8 SMC-D issues



On 25.01.24 05:59, Wen Gu wrote:
> After a while debug I found an elementary mistake of mine in
> b40584d ("net/smc: compatible with 128-bits extended GID of virtual ISM device")..
> 
> The operator order in smcd_lgr_match() is not as expected. It will always return
> 'true' in remote-system case.
> 
>  static bool smcd_lgr_match(struct smc_link_group *lgr,
> -                          struct smcd_dev *smcismdev, u64 peer_gid)
> +                          struct smcd_dev *smcismdev,
> +                          struct smcd_gid *peer_gid)
>  {
> -       return lgr->peer_gid == peer_gid && lgr->smcd == smcismdev;
> +       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;
>  }
> 
> Could you please try again with this patch? to see if this is the root cause.
> Really sorry for the inconvenience.
> 
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index da6a8d9c81ea..c6a6ba56c9e3 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1896,8 +1896,8 @@ static bool smcd_lgr_match(struct smc_link_group *lgr,
>                            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;
> +               (smc_ism_is_virtual(smcismdev) ?
> +                (lgr->peer_gid.gid_ext == peer_gid->gid_ext) : 1);
>  }
> 
> 
> Thanks,
> Wen Gu

Hello Wen Gu,

thank you for the quick resposne and for finding this nasty bug.
I can confirm that with your patch I do not see the issue anymore.
Please send a fix to the mailing lists. See
https://docs.kernel.org/process/handling-regressions.html
for some tips.

May I propose that instead of adding the brackets, you change this function 
to an if-then-else sequence for readability and maintainability?
I would still mention the missing brackets in the commit message, so
readers can quickly understand the issue.

Thanks again for the quick response.
Sandy


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ