Date: Thu, 18 Feb 2021 20:40:44 +0000 From: bugzilla-noreply@freebsd.org To: net@FreeBSD.org Subject: [Bug 253589] KCSAN race between tcp_do_segment and sbfree with vtnet Message-ID: <bug-253589-7501-tspoE2XS0p@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-253589-7501@https.bugs.freebsd.org/bugzilla/> References: <bug-253589-7501@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D253589 Jonathan T. Looney <jtl@freebsd.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jtl@freebsd.org --- Comment #2 from Jonathan T. Looney <jtl@freebsd.org> --- I can think more about this, but my initial thought is that this is probably both expected and acceptable within the context you have given. It appears that the two threads are doing these things: Thread 1: Running sbfree() to free an mbuf from a socket buffer. I am going= to surmise that this is an mbuf on the input socket buffer, and it has now been read by the application. This will decrease sb_ccc, which has the implicati= on of increasing the available space in the socket buffer. Thread 2: Running sbspace() to check on the amount of available space in a socket buffer. Because this is being called from tcp_do_segment(), I am goi= ng to surmise that this is being called from one of the two places which use sbspace() to determine the amount of data we can read from an input packet. (FWIW, even though the instruction points to the line which checks for SB_S= TOP, I assume that the actual conflict occurs on the next line of code which uses sb_ccc.) If I understand the code correctly, neither the socket nor socket buffer ca= n be freed as long as the TCP code holds a reference on the socket. And, if the = TCP code is calling tcp_do_segment(), it should be in a state where it still ha= s a reference on the socket. So, the only potential danger from the lack of synchronization is a stale read on the socket buffer values. In this particular case, I believe it is acceptable to have a stale read. T= CP advertises a receive window to its peer based on the latest information it = has about input socket buffer space. The peer should not exceed that receive window. Prior to Thread 1 modifying the sb_ccc value, the receive window wo= uld have been X. After Thread 1 modified the sb_ccc value, the receive window w= ould have been Y (where Y > X). Absent some corner cases (such as suddenly shrin= king the input buffer high water level), we should have most recently advertised= a receive window no larger than X. Therefore, whether Thread 2 reads sb_ccc before or after Thread 1 updates it, the peer should not have sent us so mu= ch data that it actually produces a functional difference. (And, if the peer d= id exceed our advertised receive window, our TCP stack is under no obligation = to accept that data.) So, for this usage within TCP, it seems to me that the locking should not m= ake a functional difference and the lack of locking for this particular read is probably a net gain. --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-253589-7501-tspoE2XS0p>