|Summary:||panic: mutex so_rcv not owned at /usr/src/sys/kern/uipc_socket.c:2359|
|Product:||Base System||Reporter:||Greg Becker <greg>|
|Component:||kern||Assignee:||Mark Johnston <markj>|
|Severity:||Affects Some People||CC:||greg, markj, net|
Description Greg Becker 2019-06-24 12:41:41 UTC
If the call to sblock() in soreceive_stream() returns non-zero we 'goto out', but since we have not yet acquired the sockbuf lock we trip the SOCKBUF_LOCK_ASSERT() at the "out" label. The following patch seems to work for me, but neglects to make the checks that are done under the sockbuf lock after the "out" label. $ svnlite diff sys/kern/uipc_socket.c Index: sys/kern/uipc_socket.c =================================================================== --- sys/kern/uipc_socket.c (revision 349288) +++ sys/kern/uipc_socket.c (working copy) @@ -2196,7 +2196,7 @@ /* Prevent other readers from entering the socket. */ error = sblock(sb, SBLOCKWAIT(flags)); if (error) - goto out; + return error; SOCKBUF_LOCK(sb); /* Easy one, no space to copyout anything. */ svnlite info Path: . Working Copy Root Path: /usr/src URL: https://svn.freebsd.org/base/stable/12 Relative URL: ^/stable/12 Repository Root: https://svn.freebsd.org/base Repository UUID: ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f Revision: 349288 Node Kind: directory Schedule: normal Last Changed Author: gjb Last Changed Rev: 349232 Last Changed Date: 2019-06-20 09:34:45 -0500 (Thu, 20 Jun 2019) [greg@sm1 src] $ svnlite status ? sys/amd64/conf/SM1 M sys/kern/uipc_socket.c
Comment 1 Kubilay Kocak 2019-06-24 13:00:08 UTC
Thank you for the report, analysis and patch Greg. Could you include your proposed change as an attachment please
Comment 2 Greg Becker 2019-06-24 13:32:16 UTC
Thanks Kubilay! Sure, give me some time to analyze the code to ensure the patch is production worthy.
Comment 3 Greg Becker 2019-06-25 12:14:32 UTC
Created attachment 205327 [details] Avert panic in soreceive_stream() This patch prevents jumping to "out" without the sockbuf lock held and tripping the SOCKBUF_LOCK_ASSERT(): panic: mutex so_rcv not owned at /usr/src/sys/kern/uipc_socket.c:2359 Since the socket buffer data was not changed I see little need to call the integrity checks at the "out" label, therefore the simple solution is to just return if sblock() returns an error.
Comment 4 Kubilay Kocak 2019-06-25 12:19:48 UTC
Thank you for the follow-up Greg. I forgot to ask originally, do you have a panic message and backtrace available? Please include it as an attachment if so
Comment 5 Greg Becker 2019-06-25 13:25:02 UTC
Sure Kubilay, here's my backtrace. It's from a kernel module I am working on that I have not yet published. I hit this easily with GENERIC, but never hit it when running with the patch I supplied: panic: mutex so_rcv not owned at /usr/src/sys/kern/uipc_socket.c:2359 cpuid = 11 time = 1561372118 KDB: stack backtrace: db_trace_self_wrapper() at db_trace_self_wrapper+0x2c/frame 0xfffffe012015e5b0 kdb_backtrace() at kdb_backtrace+0x53/frame 0xfffffe012015e660 vpanic() at vpanic+0x265/frame 0xfffffe012015e720 doadump() at doadump/frame 0xfffffe012015e780 __mtx_assert() at __mtx_assert+0x145/frame 0xfffffe012015e800 soreceive_stream() at soreceive_stream+0x963/frame 0xfffffe012015e8b0 soreceive() at soreceive+0x102/frame 0xfffffe012015e910 krpc_recv_tcp() at krpc_recv_tcp+0x55/frame 0xfffffe012015e9a0 svc_rcv_receive() at svc_rcv_receive+0x1c/frame 0xfffffe012015e9e0 tpool_run() at tpool_run+0x92/frame 0xfffffe012015ea30 fork_exit() at fork_exit+0x13b/frame 0xfffffe012015eab0 fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe012015eab0 --- trap 0, rip = 0, rsp = 0, rbp = 0 --- KDB: enter: panic [ thread pid 0 tid 101260 ] Stopped at breakpoint+0x5: popq %rbp I think what is happening is that I am post-processing a rcv soupcall, but the non-blocking call to sblock() in soreceive() loses due to a race with an asynchronously running call to soshutdown(SHUT_RD) from my module unload code. But I have not yet verified that... I'm using GENERIC with the following options: options CONSPEED=115200 options BREAK_TO_DEBUGGER options DDB options SOCKBUF_DEBUG options INVARIANTS options INVARIANT_SUPPORT FreeBSD sm1.cc.codeconcepts.com 12.0-STABLE FreeBSD 12.0-STABLE #31 r349288M: Tue Jun 25 07:19:16 CDT 2019 firstname.lastname@example.org:/usr/obj/usr/src/amd64.amd64/sys/SM1 amd64
Comment 6 Mark Johnston 2019-07-02 14:10:57 UTC
This bug has been around for a surprisingly long time. I think your patch is right.
Comment 7 Mark Johnston 2019-07-02 14:12:28 UTC
(In reply to Mark Johnston from comment #6) Ah, because soreceive_stream() is not used by default; tcp_soreceive_stream defaults to 0.
Comment 8 commit-hook 2019-07-02 14:25:22 UTC
A commit references this bug: Author: markj Date: Tue Jul 2 14:24:42 UTC 2019 New revision: 349599 URL: https://svnweb.freebsd.org/changeset/base/349599 Log: Fix handling of errors from sblock() in soreceive_stream(). Previously we would attempt to unlock the socket buffer despite having failed to lock it. Simply return an error instead: no resources need to be released at this point, and doing so is consistent with soreceive_generic(). PR: 238789 Submitted by: Greg Becker <email@example.com> MFC after: 1 week Changes: head/sys/kern/uipc_socket.c
Comment 9 commit-hook 2019-07-07 17:43:56 UTC
A commit references this bug: Author: markj Date: Sun Jul 7 17:43:15 UTC 2019 New revision: 349810 URL: https://svnweb.freebsd.org/changeset/base/349810 Log: MFC r349599: Fix handling of errors from sblock() in soreceive_stream(). PR: 238789 Changes: _U stable/12/ stable/12/sys/kern/uipc_socket.c
Comment 10 commit-hook 2019-07-07 17:43:58 UTC
A commit references this bug: Author: markj Date: Sun Jul 7 17:43:45 UTC 2019 New revision: 349811 URL: https://svnweb.freebsd.org/changeset/base/349811 Log: MFC r349599: Fix handling of errors from sblock() in soreceive_stream(). PR: 238789 Changes: _U stable/11/ stable/11/sys/kern/uipc_socket.c
Comment 11 Mark Johnston 2019-07-07 17:44:14 UTC
Thanks for the report.