[PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key().

[PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key().

[Kuniyuki Iwashima]

lockdep_sock_is_held() was added in tcp_ao_established_key()
by the cited commit.

It can be called from tcp_v[46]_timewait_ack() with twsk.

Since it does not have sk->sk_lock, the lockdep annotation
results in out-of-bound access.

  $ pahole -C tcp_timewait_sock vmlinux | grep size
  	/* size: 288, cachelines: 5, members: 8 */
  $ pahole -C sock vmlinux | grep sk_lock
  	socket_lock_t              sk_lock;              /*   440   192 */

Let's not use lockdep_sock_is_held() for TCP_TIME_WAIT.

Fixes: 6b2d11e2d8fc ("net/tcp: Add missing lockdep annotations for TCP-AO hlist traversals")
Reported-by: Damiano Melotti <melotti@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/tcp_ao.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index a97cdf3e6af4..e2720233e36b 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -116,7 +116,9 @@ struct tcp_ao_key *tcp_ao_established_key(const struct sock *sk,
 {
 	struct tcp_ao_key *key;
 
-	hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk)) {
+	hlist_for_each_entry_rcu(key, &ao->head, node,
+				 sk->sk_state == TCP_TIME_WAIT ||
+				 lockdep_sock_is_held(sk)) {
 		if ((sndid >= 0 && key->sndid != sndid) ||
 		    (rcvid >= 0 && key->rcvid != rcvid))
 			continue;
-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key().

[Eric Dumazet]

On Wed, May 6, 2026 at 10:28 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> lockdep_sock_is_held() was added in tcp_ao_established_key()
> by the cited commit.
>
> It can be called from tcp_v[46]_timewait_ack() with twsk.
>
> Since it does not have sk->sk_lock, the lockdep annotation
> results in out-of-bound access.
>
>   $ pahole -C tcp_timewait_sock vmlinux | grep size
>         /* size: 288, cachelines: 5, members: 8 */
>   $ pahole -C sock vmlinux | grep sk_lock
>         socket_lock_t              sk_lock;              /*   440   192 */
>
> Let's not use lockdep_sock_is_held() for TCP_TIME_WAIT.
>
> Fixes: 6b2d11e2d8fc ("net/tcp: Add missing lockdep annotations for TCP-AO hlist traversals")
> Reported-by: Damiano Melotti <melotti@google.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  net/ipv4/tcp_ao.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> index a97cdf3e6af4..e2720233e36b 100644
> --- a/net/ipv4/tcp_ao.c
> +++ b/net/ipv4/tcp_ao.c
> @@ -116,7 +116,9 @@ struct tcp_ao_key *tcp_ao_established_key(const struct sock *sk,
>  {
>         struct tcp_ao_key *key;
>
> -       hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk)) {
> +       hlist_for_each_entry_rcu(key, &ao->head, node,
> +                                sk->sk_state == TCP_TIME_WAIT ||
> +                                lockdep_sock_is_held(sk)) {
>                 if ((sndid >= 0 && key->sndid != sndid) ||
>                     (rcvid >= 0 && key->rcvid != rcvid))
>                         continue;

I wonder if a better fix would be to change __list_check_rcu() evaluation order.

Otherwise, this would mean that a TIME_WAIT socket would evade RCU
LOCKDEP checks.

If a fix in tcp_ao.c is te way to go, I would suggest the opposite of
what you did.

diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index a97cdf3e6af4cf1ee1cd8c6361944536056543e6..0a4b38b315fed40899901ea5f66fbdffd776df38
100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -116,7 +116,8 @@ struct tcp_ao_key *tcp_ao_established_key(const
struct sock *sk,
 {
        struct tcp_ao_key *key;

-       hlist_for_each_entry_rcu(key, &ao->head, node,
lockdep_sock_is_held(sk)) {
+       hlist_for_each_entry_rcu(key, &ao->head, node,
+                                sk_fullsock(sk) && lockdep_sock_is_held(sk)) {
                if ((sndid >= 0 && key->sndid != sndid) ||
                    (rcvid >= 0 && key->rcvid != rcvid))
                        continue;

Re: [PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key().

[Kuniyuki Iwashima]

On Thu, May 7, 2026 at 4:23 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, May 6, 2026 at 10:28 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > lockdep_sock_is_held() was added in tcp_ao_established_key()
> > by the cited commit.
> >
> > It can be called from tcp_v[46]_timewait_ack() with twsk.
> >
> > Since it does not have sk->sk_lock, the lockdep annotation
> > results in out-of-bound access.
> >
> >   $ pahole -C tcp_timewait_sock vmlinux | grep size
> >         /* size: 288, cachelines: 5, members: 8 */
> >   $ pahole -C sock vmlinux | grep sk_lock
> >         socket_lock_t              sk_lock;              /*   440   192 */
> >
> > Let's not use lockdep_sock_is_held() for TCP_TIME_WAIT.
> >
> > Fixes: 6b2d11e2d8fc ("net/tcp: Add missing lockdep annotations for TCP-AO hlist traversals")
> > Reported-by: Damiano Melotti <melotti@google.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  net/ipv4/tcp_ao.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> > index a97cdf3e6af4..e2720233e36b 100644
> > --- a/net/ipv4/tcp_ao.c
> > +++ b/net/ipv4/tcp_ao.c
> > @@ -116,7 +116,9 @@ struct tcp_ao_key *tcp_ao_established_key(const struct sock *sk,
> >  {
> >         struct tcp_ao_key *key;
> >
> > -       hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk)) {
> > +       hlist_for_each_entry_rcu(key, &ao->head, node,
> > +                                sk->sk_state == TCP_TIME_WAIT ||
> > +                                lockdep_sock_is_held(sk)) {
> >                 if ((sndid >= 0 && key->sndid != sndid) ||
> >                     (rcvid >= 0 && key->rcvid != rcvid))
> >                         continue;
>
> I wonder if a better fix would be to change __list_check_rcu() evaluation order.
>
> Otherwise, this would mean that a TIME_WAIT socket would evade RCU
> LOCKDEP checks.

Ah exactly,

>
> If a fix in tcp_ao.c is te way to go, I would suggest the opposite of
> what you did.

will change that way.

Thanks !

>
> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> index a97cdf3e6af4cf1ee1cd8c6361944536056543e6..0a4b38b315fed40899901ea5f66fbdffd776df38
> 100644
> --- a/net/ipv4/tcp_ao.c
> +++ b/net/ipv4/tcp_ao.c
> @@ -116,7 +116,8 @@ struct tcp_ao_key *tcp_ao_established_key(const
> struct sock *sk,
>  {
>         struct tcp_ao_key *key;
>
> -       hlist_for_each_entry_rcu(key, &ao->head, node,
> lockdep_sock_is_held(sk)) {
> +       hlist_for_each_entry_rcu(key, &ao->head, node,
> +                                sk_fullsock(sk) && lockdep_sock_is_held(sk)) {
>                 if ((sndid >= 0 && key->sndid != sndid) ||
>                     (rcvid >= 0 && key->rcvid != rcvid))
>                         continue;