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: <CAAVpQUA3cnUz5=2AYQ_6Od_jmHUjS0Cd20NhdzfDxB1GptfsQg@mail.gmail.com>
Date: Thu, 11 Sep 2025 09:53:39 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Eric Dumazet <edumazet@...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 10:45 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> 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 :
>

Sorry, I missed this one.  I'll drop this patch and send the
series to net-next.


> 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();
>         }
>  }