From: "Jiayuan Chen" <jiayuan.chen@linux.dev>
To: "Kuniyuki Iwashima" <kuniyu@google.com>
Cc: netdev@vger.kernel.org, "Eric Dumazet" <edumazet@google.com>,
"Paolo Abeni" <pabeni@redhat.com>,
"Willem de Bruijn" <willemb@google.com>,
"David S. Miller" <davem@davemloft.net>,
"Jakub Kicinski" <kuba@kernel.org>,
"Simon Horman" <horms@kernel.org>,
"David Ahern" <dsahern@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v1] net: annotate data races around sk->sk_prot
Date: Wed, 04 Mar 2026 03:53:46 +0000 [thread overview]
Message-ID: <0983a82f8946d972d2bf6e3a316f8556f68e3d54@linux.dev> (raw)
In-Reply-To: <CAAVpQUBn7zmPvJB+P0pyD8BAaWdc+g3warvPjahYd_Qb7x54UQ@mail.gmail.com>
March 4, 2026 at 11:42, "Kuniyuki Iwashima" <kuniyu@google.com mailto:kuniyu@google.com?to=%22Kuniyuki%20Iwashima%22%20%3Ckuniyu%40google.com%3E > wrote:
>
> On Tue, Mar 3, 2026 at 7:16 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> >
> > inet_sendmsg(), inet_recvmsg() and sock_common_recvmsg() access
> > sk->sk_prot without lock_sock() or any other synchronization.
> >
> > sock_replace_proto() (used by sockmap), TLS and MPTCP can change
> > sk->sk_prot under us, so these functions need READ_ONCE() to avoid
> > load tearing.
> >
> > Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> > ---
> > net/core/sock.c | 2 +-
> > net/ipv4/af_inet.c | 8 ++++++--
> > 2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index f4e2ff23d60e..79b659cebbb1 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -3968,7 +3968,7 @@ int sock_common_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
> > {
> > struct sock *sk = sock->sk;
> >
> > - return sk->sk_prot->recvmsg(sk, msg, size, flags);
> > + return READ_ONCE(sk->sk_prot)->recvmsg(sk, msg, size, flags);
> > }
> > EXPORT_SYMBOL(sock_common_recvmsg);
> >
> None of users seems to be supported by SOCKMAP,
> or am I missing something ?
>
> include/net/sock.h:1963:int sock_common_recvmsg(struct socket *sock,
> struct msghdr *msg, size_t size,
> net/core/sock.c:3966:int sock_common_recvmsg(struct socket *sock,
> struct msghdr *msg, size_t size,
> net/core/sock.c:3973:EXPORT_SYMBOL(sock_common_recvmsg);
> net/l2tp/l2tp_ip6.c:774: .recvmsg = sock_common_recvmsg,
> net/l2tp/l2tp_ip.c:645: .recvmsg = sock_common_recvmsg,
> net/ipv6/raw.c:1292: .recvmsg = sock_common_recvmsg, /* ok */
> net/ieee802154/socket.c:427: .recvmsg = sock_common_recvmsg,
> net/ieee802154/socket.c:989: .recvmsg = sock_common_recvmsg,
> net/phonet/socket.c:441: .recvmsg = sock_common_recvmsg,
> net/phonet/socket.c:461: .recvmsg = sock_common_recvmsg,
You're right. None of the sock_common_recvmsg() users (raw, l2tp,
ieee802154, phonet) support SOCKMAP/TLS/MPTCP, so there is no
concurrent writer to sk->sk_prot for these socket types. I'll drop
that change in v2.
> >
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index babcd75a08e2..e95ffa070568 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -852,11 +852,13 @@ EXPORT_SYMBOL_GPL(inet_send_prepare);
> > int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
> > {
> > struct sock *sk = sock->sk;
> > + const struct proto *prot;
> >
> > if (unlikely(inet_send_prepare(sk)))
> > return -EAGAIN;
> >
> > - return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udp_sendmsg,
> > + prot = READ_ONCE(sk->sk_prot);
> > + return INDIRECT_CALL_2(prot->sendmsg, tcp_sendmsg, udp_sendmsg,
> > sk, msg, size);
> > }
> > EXPORT_SYMBOL(inet_sendmsg);
> > @@ -882,11 +884,13 @@ int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
> > int flags)
> > {
> > struct sock *sk = sock->sk;
> > + const struct proto *prot;
> >
> > if (likely(!(flags & MSG_ERRQUEUE)))
> > sock_rps_record_flow(sk);
> >
> > - return INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udp_recvmsg,
> > + prot = READ_ONCE(sk->sk_prot);
> > + return INDIRECT_CALL_2(prot->recvmsg, tcp_recvmsg, udp_recvmsg,
> > sk, msg, size, flags);
> > }
> > EXPORT_SYMBOL(inet_recvmsg);
> > --
> > 2.43.0
> >
>
prev parent reply other threads:[~2026-03-04 3:53 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-04 3:16 [PATCH net-next v1] net: annotate data races around sk->sk_prot Jiayuan Chen
2026-03-04 3:42 ` Kuniyuki Iwashima
2026-03-04 3:53 ` Jiayuan Chen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0983a82f8946d972d2bf6e3a316f8556f68e3d54@linux.dev \
--to=jiayuan.chen@linux.dev \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=kuniyu@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=willemb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox