diff mbox series

[net] net: ipv6: fix skb hash for some RST packets

Message ID 20230427092159.44998-1-atenart@kernel.org (mailing list archive)
State Accepted
Commit dc6456e938e938d64ffb6383a286b2ac9790a37f
Delegated to: Netdev Maintainers
Headers show
Series [net] net: ipv6: fix skb hash for some RST packets | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 10 this patch: 10
netdev/cc_maintainers warning 1 maintainers not CCed: dsahern@kernel.org
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 10 this patch: 10
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Antoine Tenart April 27, 2023, 9:21 a.m. UTC
The skb hash comes from sk->sk_txhash when using TCP, except for some
IPv6 RST packets. This is because in tcp_v6_send_reset when not in
TIME_WAIT the hash is taken from sk->sk_hash, while it should come from
sk->sk_txhash as those two hashes are not computed the same way.

Packetdrill script to test the above,

   0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
  +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
  +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)

  +0 > (flowlabel 0x1) S 0:0(0) <...>

  // Wrong ack seq, trigger a rst.
  +0 < S. 0:0(0) ack 0 win 4000

  // Check the flowlabel matches prior one from SYN.
  +0 > (flowlabel 0x1) R 0:0(0) <...>

Fixes: 9258b8b1be2e ("ipv6: tcp: send consistent autoflowlabel in RST packets")
Signed-off-by: Antoine Tenart <atenart@kernel.org>
---
 net/ipv6/tcp_ipv6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eric Dumazet April 27, 2023, 1:23 p.m. UTC | #1
On Thu, Apr 27, 2023 at 11:22 AM Antoine Tenart <atenart@kernel.org> wrote:
>
> The skb hash comes from sk->sk_txhash when using TCP, except for some
> IPv6 RST packets. This is because in tcp_v6_send_reset when not in
> TIME_WAIT the hash is taken from sk->sk_hash, while it should come from
> sk->sk_txhash as those two hashes are not computed the same way.
>
> Packetdrill script to test the above,
>
>    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
>   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
>   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
>
>   +0 > (flowlabel 0x1) S 0:0(0) <...>
>
>   // Wrong ack seq, trigger a rst.
>   +0 < S. 0:0(0) ack 0 win 4000
>
>   // Check the flowlabel matches prior one from SYN.
>   +0 > (flowlabel 0x1) R 0:0(0) <...>
>
> Fixes: 9258b8b1be2e ("ipv6: tcp: send consistent autoflowlabel in RST packets")
> Signed-off-by: Antoine Tenart <atenart@kernel.org>
> ---

Good catch, thanks !

Reviewed-by: Eric Dumazet <edumazet@google.com>
patchwork-bot+netdevbpf@kernel.org April 28, 2023, 9 a.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 27 Apr 2023 11:21:59 +0200 you wrote:
> The skb hash comes from sk->sk_txhash when using TCP, except for some
> IPv6 RST packets. This is because in tcp_v6_send_reset when not in
> TIME_WAIT the hash is taken from sk->sk_hash, while it should come from
> sk->sk_txhash as those two hashes are not computed the same way.
> 
> Packetdrill script to test the above,
> 
> [...]

Here is the summary with links:
  - [net] net: ipv6: fix skb hash for some RST packets
    https://git.kernel.org/netdev/net/c/dc6456e938e9

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 1bf93b61aa06..e2898912c90c 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1064,7 +1064,7 @@  static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
 			if (np->repflow)
 				label = ip6_flowlabel(ipv6h);
 			priority = sk->sk_priority;
-			txhash = sk->sk_hash;
+			txhash = sk->sk_txhash;
 		}
 		if (sk->sk_state == TCP_TIME_WAIT) {
 			label = cpu_to_be32(inet_twsk(sk)->tw_flowlabel);