Bug 238789 - panic: mutex so_rcv not owned at /usr/src/sys/kern/uipc_socket.c:2359
Summary: panic: mutex so_rcv not owned at /usr/src/sys/kern/uipc_socket.c:2359
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.0-STABLE
Hardware: amd64 Any
: --- Affects Some People
Assignee: Mark Johnston
URL:
Keywords: panic
Depends on:
Blocks:
 
Reported: 2019-06-24 12:41 UTC by Greg Becker
Modified: 2019-07-09 02:41 UTC (History)
3 users (show)

See Also:
koobs: mfc-stable11+
koobs: mfc-stable12+


Attachments
Avert panic in soreceive_stream() (430 bytes, patch)
2019-06-25 12:14 UTC, Greg Becker
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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     greg@sm1.cc.codeconcepts.com:/usr/obj/usr/src/amd64.amd64/sys/SM1  amd64
Comment 6 Mark Johnston freebsd_committer 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 freebsd_committer 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 freebsd_committer 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 <greg@codeconcepts.com>
  MFC after:	1 week

Changes:
  head/sys/kern/uipc_socket.c
Comment 9 commit-hook freebsd_committer 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 freebsd_committer 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 freebsd_committer 2019-07-07 17:44:14 UTC
Thanks for the report.