linux-kernel - Re: [PATCH net-next v2 2/2] net/smc: support ipv4 mapped ipv6 addr client for smc-r v2

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: <20241205135833.0beafd61.pasic@linux.ibm.com>
Date: Thu, 5 Dec 2024 13:58:33 +0100
From: Halil Pasic <pasic@...ux.ibm.com>
To: Wenjia Zhang <wenjia@...ux.ibm.com>
Cc: Guangguan Wang <guangguan.wang@...ux.alibaba.com>, jaka@...ux.ibm.com,
        alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
        guwen@...ux.alibaba.com, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
        linux-rdma@...r.kernel.org, linux-s390@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Dust Li
 <dust.li@...ux.alibaba.com>,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: [PATCH net-next v2 2/2] net/smc: support ipv4 mapped ipv6 addr
 client for smc-r v2

On Thu, 5 Dec 2024 11:16:27 +0100
Wenjia Zhang <wenjia@...ux.ibm.com> wrote:

> > --- a/net/smc/af_smc.c
> > +++ b/net/smc/af_smc.c
> > @@ -1116,7 +1116,12 @@ static int smc_find_proposal_devices(struct
> > smc_sock *smc, ini->check_smcrv2 = true;
> >   	ini->smcrv2.saddr = smc->clcsock->sk->sk_rcv_saddr;
> >   	if (!(ini->smcr_version & SMC_V2) ||
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +	    (smc->clcsock->sk->sk_family != AF_INET &&
> > +
> > !ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)) ||  
> I think here you want to say !(smc->clcsock->sk->sk_family == AF_INET
> && ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)), right? If
> it is, the negativ form of the logical operation (a&&b) is (!a)||(!b),
> i.e. here should be:
> （smc->clcsock->sk->sk_family != AF_INET）|| 
> （!ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)）

Wenjia, I think you happen to confuse something here. The condition
of this if statement is supposed to evaluate as true iff we don't want
to propose SMCRv2 because the situation is such that SMCRv2 is not
supported.

We have a bunch of conditions we need to meet for SMCRv2 so
logically we have (A && B && C && D). Now since the if is
about when SMCRv2 is not supported we have a super structure
that looks like !A || !B || !C || !D. With this patch, if
CONFIG_IPV6 is not enabled, the sub-condition remains the same:
if smc->clcsock->sk->sk_family is something else that AF_INET
the we do not do SMCRv2!

But when we do have CONFIG_IPV6 then we want to do SMCRv2 for
AF_INET6 sockets too if the addresses used are actually
v4 mapped addresses.

Now this is where the cognitive dissonance starts on my end. I
think the author assumes sk_family == AF_INET || sk_family == AF_INET6
is a tautology in this context. That may be a reasonable thing to
assume. Under that assumption 
sk_family != AF_INET &&	!ipv6_addr_v4mapped(addr) (shortened for
convenience)
becomes equivalent to
sk_family == AF_INET6 && !ipv6_addr_v4mapped(addr)
which means in words if the socket is an IPv6 sockeet and the addr is not
a v4 mapped v6 address then we *can not* do SMCRv2. And the condition
when we can is sk_family != AF_INET6 || ipv6_addr_v4mapped(addr) which
is equivalen to sk_family == AF_INET || ipv6_addr_v4mapped(addr) under
the aforementioned assumption.

But if we assume sk_family == AF_INET || sk_family == AF_INET6 then
the #else does not make any sense, because I guess with IPv6 not
available AF_INET6 is not available ant thus the else is always
guaranteed to evaluate to false under the assumption made.

Thus I conclude, that I am certainly missing something here. Guangguan,
do you care to explain?

Regards,
Halil