On Tue, Mar 16, 2021 at 10:41 AM Pavel Machek pavel@denx.de wrote:
Hi!
From: Greg Kroah-Hartman gregkh@linuxfoundation.org
From: Eric Dumazet edumazet@google.com
Two From: fields here.
[ Upstream commit 7db48e983930285b765743ebd665aecf9850582b ]
There are few places where we fetch tp->copied_seq while this field can change from IRQ or other cpu.
And there are few such places even after the patch is applied; I quoted them below.
Doing addition to variable without locking... is kind of interesting. Are you sure it is okay?
We are holding the socket lock here.
The WRITE_ONCE() here is paired with sides doing READ_ONCE() while socket lock is _not_ held.
We want to make sure compiler won't write into this variable one byte at a time, or using stupid things.
@@ -2112,7 +2112,7 @@ int tcp_recvmsg(struct sock *sk, struct if (urg_offset < used) { if (!urg_offset) { if (!sock_flag(sk, SOCK_URGINLINE)) {
++*seq;
WRITE_ONCE(*seq, *seq + 1); urg_hole++; offset++; used--;@@ -2134,7 +2134,7 @@ int tcp_recvmsg(struct sock *sk, struct } }
*seq += used;
WRITE_ONCE(*seq, *seq + used); copied += used; len -= used;@@ -2163,7 +2163,7 @@ skip_copy:
found_fin_ok: /* Process the FIN. */
++*seq;
WRITE_ONCE(*seq, *seq + 1); if (!(flags & MSG_PEEK)) sk_eat_skb(sk, skb); break;Best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany