netdev - Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo

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: <CANn89iJnOZ-bYc9bAPSphiCX4vwy4af2r_QvF1ukVXF7DyA4Kw@mail.gmail.com>
Date: Wed, 10 Sep 2025 22:45:31 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, 
	Neal Cardwell <ncardwell@...gle.com>
Subject: Re: [PATCH v1 net 6/8] tcp: Use sk_dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check().

On Wed, Sep 10, 2025 at 8:06 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
>
> tcp_fastopen_active_disable_ofo_check() is called from tcp_disconnect()
> or tcp_v4_destroy_sock(), so not under RCU nor RTNL.
>
> Using sk_dst_get(sk)->dev could trigger UAF.
>
> Let's use sk_dst_dev_rcu().
>
> Fixes: 4a6ce2b6f2ec ("net: introduce a new function dst_dev_put()")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> ---
> Cc: Neal Cardwell <ncardwell@...gle.com>
> ---
>  net/ipv4/tcp_fastopen.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index f1884f0c9e52..de849b8c87ef 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -560,7 +560,6 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>         struct net_device *dev;
> -       struct dst_entry *dst;
>         struct sk_buff *skb;
>
>         if (!tp->syn_fastopen)
> @@ -576,11 +575,11 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
>                 }
>         } else if (tp->syn_fastopen_ch &&
>                    atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
> -               dst = sk_dst_get(sk);
> -               dev = dst ? dst_dev(dst) : NULL;
> +               rcu_read_lock();
> +               dev = sk_dst_dev_rcu(sk);
>                 if (!(dev && (dev->flags & IFF_LOOPBACK)))
>                         atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
> -               dst_release(dst);
> +               rcu_read_unlock();
>         }
>  }
>
> --
> 2.51.0.384.g4c02a37b29-goog
>

NACK. Please send a series against net-next as I did recently.

You will then discover :

commit b62a59c18b692f892dcb8109c1c2e653b2abc95c
Author: Eric Dumazet <edumazet@...gle.com>
Date:   Thu Aug 28 19:58:22 2025 +0000

    tcp: use dst_dev_rcu() in tcp_fastopen_active_disable_ofo_check()

    Use RCU to avoid a pair of atomic operations and a potential
    UAF on dst_dev()->flags.

    Signed-off-by: Eric Dumazet <edumazet@...gle.com>
    Reviewed-by: David Ahern <dsahern@...nel.org>
    Link: https://patch.msgid.link/20250828195823.3958522-8-edumazet@google.com
    Signed-off-by: Jakub Kicinski <kuba@...nel.org>

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index f1884f0c9e523d50b2d120175cc94bc40b489dfb..7d945a527daf093f87882c7949e21058ed6df1cc
100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -576,11 +576,12 @@ void
tcp_fastopen_active_disable_ofo_check(struct sock *sk)
                }
        } else if (tp->syn_fastopen_ch &&
                   atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
-               dst = sk_dst_get(sk);
-               dev = dst ? dst_dev(dst) : NULL;
+               rcu_read_lock();
+               dst = __sk_dst_get(sk);
+               dev = dst ? dst_dev_rcu(dst) : NULL;
                if (!(dev && (dev->flags & IFF_LOOPBACK)))

atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
-               dst_release(dst);
+               rcu_read_unlock();
        }
 }