* [PATCH] tcp: fix secure_{tcp, tcpv6}_ts_off call parameter order mistake
@ 2023-10-07 9:23 George Guo
2023-10-07 10:50 ` Florian Westphal
0 siblings, 1 reply; 4+ messages in thread
From: George Guo @ 2023-10-07 9:23 UTC (permalink / raw)
To: edumazet, davem, dsahern, kuba, pabeni; +Cc: netdev, linux-kernel, George Guo
From: George Guo <guodongtai@kylinos.cn>
Fix secure_tcp_ts_off and secure_tcpv6_ts_off call parameter order mistake
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 27140e5cdc06..3d6c9b286b5a 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 3a88545a265d..ce9cc4c43cf2 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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] tcp: fix secure_{tcp, tcpv6}_ts_off call parameter order mistake 2023-10-07 9:23 [PATCH] tcp: fix secure_{tcp, tcpv6}_ts_off call parameter order mistake George Guo @ 2023-10-07 10:50 ` Florian Westphal 2023-10-09 2:11 ` [PATCH v2] tcp: cleanup secure_{tcp, tcpv6}_ts_off George Guo 0 siblings, 1 reply; 4+ messages in thread From: Florian Westphal @ 2023-10-07 10:50 UTC (permalink / raw) To: George Guo Cc: edumazet, davem, dsahern, kuba, pabeni, netdev, linux-kernel, George Guo George Guo <dongtai.guo@linux.dev> wrote: > From: George Guo <guodongtai@kylinos.cn> > > Fix secure_tcp_ts_off and secure_tcpv6_ts_off call parameter order mistake Could you please send a v2, targetting net-next, that clearly says that this is a cleanup? [ It doesn't matter if we pass "saddr, daddr" or "daddr, saddr" as long as both "init" and "check" functions agree on the ordering ] ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] tcp: cleanup secure_{tcp, tcpv6}_ts_off 2023-10-07 10:50 ` Florian Westphal @ 2023-10-09 2:11 ` George Guo 2023-10-09 7:18 ` Eric Dumazet 0 siblings, 1 reply; 4+ messages in thread From: George Guo @ 2023-10-09 2:11 UTC (permalink / raw) To: fw Cc: davem, dongtai.guo, dsahern, edumazet, guodongtai, kuba, linux-kernel, netdev, pabeni Correct secure_tcp_ts_off and secure_tcpv6_ts_off call parameter order 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] tcp: cleanup secure_{tcp, tcpv6}_ts_off 2023-10-09 2:11 ` [PATCH v2] tcp: cleanup secure_{tcp, tcpv6}_ts_off George Guo @ 2023-10-09 7:18 ` Eric Dumazet 0 siblings, 0 replies; 4+ messages in thread From: Eric Dumazet @ 2023-10-09 7:18 UTC (permalink / raw) To: George Guo Cc: fw, davem, dongtai.guo, dsahern, kuba, linux-kernel, netdev, pabeni 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-09 7:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-07 9:23 [PATCH] tcp: fix secure_{tcp, tcpv6}_ts_off call parameter order mistake George Guo
2023-10-07 10:50 ` Florian Westphal
2023-10-09 2:11 ` [PATCH v2] tcp: cleanup secure_{tcp, tcpv6}_ts_off George Guo
2023-10-09 7:18 ` Eric Dumazet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).