| 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
| ||
|
Message-ID: <CANn89i+_DoEDcFY9SfNqQ+8bqJ0kFpt4waQ8CSvhchE4aP2Dhw@mail.gmail.com>
Date: Fri, 4 Aug 2023 16:57:04 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: "Jason A. Donenfeld" <Jason@...c4.com>
Cc: Pavel Begunkov <asml.silence@...il.com>, netdev@...r.kernel.org, davem@...emloft.net,
dsahern@...nel.org, pabeni@...hat.com, kuba@...nel.org
Subject: Re: [PATCH net-next] net/tcp: refactor tcp_inet6_sk()
On Fri, Aug 4, 2023 at 4:51 PM Jason A. Donenfeld <Jason@...c4.com> wrote:
>
> Hi Pavel,
>
> On Fri, May 19, 2023 at 02:30:36PM +0100, Pavel Begunkov wrote:
> > Don't keep hand coded offset caluclations and replace it with
> > container_of(). It should be type safer and a bit less confusing.
> >
> > It also makes it with a macro instead of inline function to preserve
> > constness, which was previously casted out like in case of
> > tcp_v6_send_synack().
> >
> > Signed-off-by: Pavel Begunkov <asml.silence@...il.com>
> > ---
> > net/ipv6/tcp_ipv6.c | 10 +++-------
> > 1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 7132eb213a7a..d657713d1c71 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -93,12 +93,8 @@ static struct tcp_md5sig_key *tcp_v6_md5_do_lookup(const struct sock *sk,
> > * This avoids a dereference and allow compiler optimizations.
> > * It is a specialized version of inet6_sk_generic().
> > */
> > -static struct ipv6_pinfo *tcp_inet6_sk(const struct sock *sk)
> > -{
> > - unsigned int offset = sizeof(struct tcp6_sock) - sizeof(struct ipv6_pinfo);
> > -
> > - return (struct ipv6_pinfo *)(((u8 *)sk) + offset);
> > -}
> > +#define tcp_inet6_sk(sk) (&container_of_const(tcp_sk(sk), \
> > + struct tcp6_sock, tcp)->inet6)
> >
> > static void inet6_sk_rx_dst_set(struct sock *sk, const struct sk_buff *skb)
> > {
> > @@ -533,7 +529,7 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
> > struct sk_buff *syn_skb)
> > {
> > struct inet_request_sock *ireq = inet_rsk(req);
> > - struct ipv6_pinfo *np = tcp_inet6_sk(sk);
> > + const struct ipv6_pinfo *np = tcp_inet6_sk(sk);
> > struct ipv6_txoptions *opt;
> > struct flowi6 *fl6 = &fl->u.ip6;
> > struct sk_buff *skb;
> > --
> > 2.40.0
>
> This patch broke the WireGuard test suite on 32-bit platforms:
>
> https://build.wireguard.com/wireguard-linux-stable/bf400e83708d055bdf442577ed2f2a8eb87a06f2/i686.log
> https://build.wireguard.com/wireguard-linux-stable/bf400e83708d055bdf442577ed2f2a8eb87a06f2/arm.log
> https://build.wireguard.com/wireguard-linux-stable/bf400e83708d055bdf442577ed2f2a8eb87a06f2/armeb.log
> https://build.wireguard.com/wireguard-linux-stable/bf400e83708d055bdf442577ed2f2a8eb87a06f2/powerpc.log
> https://build.wireguard.com/wireguard-linux-stable/bf400e83708d055bdf442577ed2f2a8eb87a06f2/mips.log
> https://build.wireguard.com/wireguard-linux-stable/bf400e83708d055bdf442577ed2f2a8eb87a06f2/mipsel.log
>
> The common point of failure in each of these is something like:
>
> [+] NS1: iperf3 -s -1 -B fd00::1
> [+] NS1: wait for iperf:5201 pid 115
> -----------------------------------------------------------
> Server listening on 5201 (test #1)
> -----------------------------------------------------------
> [+] NS2: iperf3 -Z -t 3 -c fd00::1
> [ 8.908396] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 9.955882] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 10.994917] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 12.034269] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 13.073905] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 14.114022] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 16.194810] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 19.074925] wireguard: wg0: Sending keepalive packet to peer 1 (127.0.0.1:2)
> [ 19.075934] wireguard: wg0: Receiving keepalive packet from peer 2 (127.0.0.1:1)
> [ 20.273212] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 28.682020] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 30.593430] wireguard: wg0: Sending keepalive packet to peer 1 (127.0.0.1:2)
> [ 30.595999] wireguard: wg0: Receiving keepalive packet from peer 2 (127.0.0.1:1)
> [ 45.315640] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 55.560359] wireguard: wg0: Sending keepalive packet to peer 1 (127.0.0.1:2)
> [ 55.561675] wireguard: wg0: Receiving keepalive packet from peer 2 (127.0.0.1:1)
> [ 77.961218] wireguard: wg0: Packet has unallowed src IP (::2:0:0) from peer 1 (127.0.0.1:2)
> [ 88.200150] wireguard: wg0: Sending keepalive packet to peer 1 (127.0.0.1:2)
> [ 88.201031] wireguard: wg0: Receiving keepalive packet from peer 2 (127.0.0.1:1)
> iperf3: error - unable to connect to server: Operation timed out
>
> For some strange reason, the packets appear to have a src IP of
> "::2:0:0" instead of fd00::2. It looks like some kind of offset issue, I
> suppose. So you may want to revert this or reevaluate the calculation of
> `offset` here, as there's something screwy happening on 32-bit systems.
>
> Jason
I think my patch fixed this issue, can you double check ?
f5f80e32de12fad2813d37270e8364a03e6d3ef0 ipv6: remove hard coded
limitation on ipv6_pinfo
I was not sure if Pavel was problematic or not, I only guessed.
Powered by blists - more mailing lists