| Message ID | tencent_DC4901C65DB41D86CD613F3ACC41EA143808@qq.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: mark racy access on sk->sk_rcvbuf | expand |
On Sat, Mar 9, 2024 at 2:35 PM linke li <lilinke99@qq.com> wrote: > > sk->sk_rcvbuf can be changed by other threads. Mark this as benign using > READ_ONCE. > > This patch is aimed at reducing the number of benign races reported by > KCSAN in order to focus future debugging effort on harmful races. > > Signed-off-by: linke li <lilinke99@qq.com> > --- > net/core/sock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/sock.c b/net/core/sock.c > index 5e78798456fd..4c5524e70534 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -551,7 +551,7 @@ int __sk_receive_skb(struct sock *sk, struct sk_buff *skb, > > skb->dev = NULL; > > - if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { > + if (sk_rcvqueues_full(sk, READ_ONCE(sk->sk_rcvbuf))) { > atomic_inc(&sk->sk_drops); > goto discard_and_relse; OK, but what about __sock_queue_rcv_skb() in the same file ?
> OK, but what about __sock_queue_rcv_skb() in the same file ?
I notice that, but I am not very sure whether there is a data race. If it
is a similar situation, then the same patch should be applied too.
On Sat, Mar 9, 2024 at 10:10 PM linke li <lilinke99@qq.com> wrote: > > > OK, but what about __sock_queue_rcv_skb() in the same file ? > > I notice that, but I am not very sure whether there is a data race. If it > is a similar situation, then the same patch should be applied too. During that process, I see no lock owning the socket, so sk->sk_rcvbuf should also be read locklessly. Thanks, Jason > >
diff --git a/net/core/sock.c b/net/core/sock.c index 5e78798456fd..4c5524e70534 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -551,7 +551,7 @@ int __sk_receive_skb(struct sock *sk, struct sk_buff *skb, skb->dev = NULL; - if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { + if (sk_rcvqueues_full(sk, READ_ONCE(sk->sk_rcvbuf))) { atomic_inc(&sk->sk_drops); goto discard_and_relse; }
sk->sk_rcvbuf can be changed by other threads. Mark this as benign using READ_ONCE. This patch is aimed at reducing the number of benign races reported by KCSAN in order to focus future debugging effort on harmful races. Signed-off-by: linke li <lilinke99@qq.com> --- net/core/sock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)