Messages in this thread Patch in this message | | | From | Eric Dumazet <> | Date | Mon, 9 Oct 2023 09:18:58 +0200 | Subject | Re: [PATCH v2] tcp: cleanup secure_{tcp, tcpv6}_ts_off |
| |
On Mon, Oct 9, 2023 at 5:12 AM George Guo <guodongtai@kylinos.cn> wrote: > > Correct secure_tcp_ts_off and secure_tcpv6_ts_off call parameter order >
I do not think this patch is correct.
We have to exchange saddr/daddr from an incoming packet in order to compute a hash if the function expects saddr to be the local host address, and daddr being the remote peer address.
For instance, tcp_v4_connect() uses :
WRITE_ONCE(tp->tsoffset, secure_tcp_ts_off(net, inet->inet_saddr, inet->inet_daddr));
While when receiving a packet from the other peer, it correctly swaps saddr/daddr
tcp_v4_init_ts_off(const struct net *net, const struct sk_buff *skb) { return secure_tcp_ts_off(net, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr); }
> Signed-off-by: George Guo <guodongtai@kylinos.cn> > --- > net/ipv4/syncookies.c | 4 ++-- > net/ipv4/tcp_ipv4.c | 2 +- > net/ipv6/syncookies.c | 4 ++-- > net/ipv6/tcp_ipv6.c | 4 ++-- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c > index dc478a0574cb..537f368a0b66 100644 > --- a/net/ipv4/syncookies.c > +++ b/net/ipv4/syncookies.c > @@ -360,8 +360,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb) > > if (tcp_opt.saw_tstamp && tcp_opt.rcv_tsecr) { > tsoff = secure_tcp_ts_off(sock_net(sk), > - ip_hdr(skb)->daddr, > - ip_hdr(skb)->saddr); > + ip_hdr(skb)->saddr, > + ip_hdr(skb)->daddr); > tcp_opt.rcv_tsecr -= tsoff; > } > > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c > index a441740616d7..54717d261693 100644 > --- a/net/ipv4/tcp_ipv4.c > +++ b/net/ipv4/tcp_ipv4.c > @@ -104,7 +104,7 @@ static u32 tcp_v4_init_seq(const struct sk_buff *skb) > > static u32 tcp_v4_init_ts_off(const struct net *net, const struct sk_buff *skb) > { > - return secure_tcp_ts_off(net, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr); > + return secure_tcp_ts_off(net, ip_hdr(skb)->saddr, ip_hdr(skb)->daddr); > } > > int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp) > diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c > index 5014aa663452..9af484a4d518 100644 > --- a/net/ipv6/syncookies.c > +++ b/net/ipv6/syncookies.c > @@ -162,8 +162,8 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) > > if (tcp_opt.saw_tstamp && tcp_opt.rcv_tsecr) { > tsoff = secure_tcpv6_ts_off(sock_net(sk), > - ipv6_hdr(skb)->daddr.s6_addr32, > - ipv6_hdr(skb)->saddr.s6_addr32); > + ipv6_hdr(skb)->saddr.s6_addr32, > + ipv6_hdr(skb)->daddr.s6_addr32); > tcp_opt.rcv_tsecr -= tsoff; > } > > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c > index bfe7d19ff4fd..7e2f924725c6 100644 > --- a/net/ipv6/tcp_ipv6.c > +++ b/net/ipv6/tcp_ipv6.c > @@ -119,8 +119,8 @@ static u32 tcp_v6_init_seq(const struct sk_buff *skb) > > static u32 tcp_v6_init_ts_off(const struct net *net, const struct sk_buff *skb) > { > - return secure_tcpv6_ts_off(net, ipv6_hdr(skb)->daddr.s6_addr32, > - ipv6_hdr(skb)->saddr.s6_addr32); > + return secure_tcpv6_ts_off(net, ipv6_hdr(skb)->saddr.s6_addr32, > + ipv6_hdr(skb)->daddr.s6_addr32); > } > > static int tcp_v6_pre_connect(struct sock *sk, struct sockaddr *uaddr, > -- > 2.34.1 >
| |