* [PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key().
@ 2026-05-06 17:28 Kuniyuki Iwashima
2026-05-07 11:23 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-06 17:28 UTC (permalink / raw)
To: Eric Dumazet, Neal Cardwell, David S. Miller, Jakub Kicinski,
Paolo Abeni
Cc: Dmitry Safonov, Simon Horman, Kuniyuki Iwashima,
Kuniyuki Iwashima, netdev, Damiano Melotti
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key().
2026-05-06 17:28 [PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key() Kuniyuki Iwashima
@ 2026-05-07 11:23 ` Eric Dumazet
2026-05-07 22:42 ` Kuniyuki Iwashima
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2026-05-07 11:23 UTC (permalink / raw)
To: Kuniyuki Iwashima, Paul E. McKenney
Cc: Neal Cardwell, David S. Miller, Jakub Kicinski, Paolo Abeni,
Dmitry Safonov, Simon Horman, Kuniyuki Iwashima, netdev,
Damiano Melotti
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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key().
2026-05-07 11:23 ` Eric Dumazet
@ 2026-05-07 22:42 ` Kuniyuki Iwashima
0 siblings, 0 replies; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-05-07 22:42 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paul E. McKenney, Neal Cardwell, David S. Miller, Jakub Kicinski,
Paolo Abeni, Dmitry Safonov, Simon Horman, Kuniyuki Iwashima,
netdev, Damiano Melotti
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;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-07 22:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 17:28 [PATCH v1 net] tcp: Fix out-of-bounds access for twsk in tcp_ao_established_key() Kuniyuki Iwashima
2026-05-07 11:23 ` Eric Dumazet
2026-05-07 22:42 ` Kuniyuki Iwashima
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox