Bug 238814 - geom: topology lock being dropped in dumpconf of gate, raid, & raid3
Summary: geom: topology lock being dropped in dumpconf of gate, raid, & raid3
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-geom (Nobody)
Keywords: crash, needs-patch, needs-qa
Depends on:
Reported: 2019-06-26 01:47 UTC by Ryan Libby
Modified: 2019-06-26 21:28 UTC (History)
1 user (show)

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


Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Libby freebsd_committer 2019-06-26 01:47:01 UTC
These g_geom->dumpconf implementations are dropping and reacquiring the
topology lock:
 - g_gate_dumpconf()
 - g_raid_dumpconf()
 - g_raid3_dumpconf()

I think that is unsafe in two ways.  First, as soon as the topology lock
is dropped, the state may be invalidated.  In particular the softc may
be destroyed.  Second, the dumpconf procedures are called during
topology iteration (g_conf{txt,dot,xml}), and dropping the lock may also
invalidate those iterations.

I suspect that none of the above actually need to drop the topology
lock, but I haven't read each carefully yet.  For raid and raid3, they
seem to be doing it for lock order with the softc lock, but I wonder if
they don't need the softc lock at all.  And for gate, the lock order is
already compatible (topology before the g_gate_units_lock, although I
think perhaps it doesn't even need the hold/release).

Here's an example crash with debug logging turned up:

sysctl kern.geom.raid3.debug=4
sysctl debug.fail_point.mnowait=1%return
while true; do kyua test -k /usr/tests/sys/geom/class/raid3/Kyuafile; done

db> show msgbuf
msgbufp = 0xfffff8023fffffb8
magic = 63062, size = 98232, r= 1388339, w = 1390017, ptr = 0xfffff8023ffe8000, cksum= 7799430

GEOM_RAID3[2]: Access md9 r-1w-1e-1 = 0
GEOM_RAID3[2]: Metadata on md12 updated.
GEOM_RAID3[2]: Consumer md12 destroyed.
GEOM_RAID3[2]: Access md12 r-1w-1e-1 = 0
GEOM_RAID3[0]: Device graid3.Z4UWAd destroyed.
[1]: Thread exiting.

Fatal trap 9: general protection fault while in kernel mode
cpuid = 7; apic id = 07
instruction pointer     = 0x20:0xffffffff80ba7774
stack pointer           = 0x28:0xfffffe00005708c0
frame pointer           = 0x28:0xfffffe0000570960
code segment            = base rx0, limit 0xfffff, type 0x1b
                        = DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags        = interrupt enabled, resume, IOPL = 0
current process         = 13 (g_event)
trap number             = 9
panic: general protection fault
cpuid = 7
time = 1561493868
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe00005705d0
vpanic() at vpanic+0x19d/frame 0xfffffe0000570620
panic() at panic+0x43/frame 0xfffffe0000570680
trap_fatal() at trap_fatal+0x39c/frame 0xfffffe00005706e0
trap() at trap+0x6c/frame 0xfffffe00005707f0
calltrap() at calltrap+0x8/frame 0xfffffe00005707f0
--- trap 0x9, rip = 0xffffffff80ba7774, rsp = 0xfffffe00005708c0, rbp = 0xfffffe0000570960 ---
_sx_xlock_hard() at _sx_xlock_hard+0x274/frame 0xfffffe0000570960
_sx_xlock() at _sx_xlock+0xc1/frame 0xfffffe00005709a0
g_raid3_dumpconf() at g_raid3_dumpconf+0x11c/frame 0xfffffe00005709e0
g_conf_specific() at g_conf_specific+0x1aa/frame 0xfffffe0000570a30
g_run_events() at g_run_events+0x197/frame 0xfffffe0000570a70
fork_exit() at fork_exit+0x84/frame 0xfffffe0000570ab0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe0000570ab0
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---
KDB: enter: panic

db> bt 100214
Tracing pid 2864 tid 100214 td 0xfffff80003c8e5a0
sched_switch() at sched_switch+0x713/frame 0xfffffe00512b7560
mi_switch() at mi_switch+0x174/frame 0xfffffe00512b7590
sleepq_switch() at sleepq_switch+0x110/frame 0xfffffe00512b75d0
sleepq_timedwait() at sleepq_timedwait+0x4f/frame 0xfffffe00512b7610
_sleep() at _sleep+0x279/frame 0xfffffe00512b76b0
g_waitfor_event() at g_waitfor_event+0xe3/frame 0xfffffe00512b7740
sysctl_kern_geom_confxml() at sysctl_kern_geom_confxml+0x39/frame 0xfffffe00512b7770
sysctl_root_handler_locked() at sysctl_root_handler_locked+0x7b/frame 0xfffffe00512b77b0
sysctl_root() at sysctl_root+0x20c/frame 0xfffffe00512b7830
userland_sysctl() at userland_sysctl+0x17b/frame 0xfffffe00512b78e0
sys___sysctl() at sys___sysctl+0x5f/frame 0xfffffe00512b7990
amd64_syscall() at amd64_syscall+0x276/frame 0xfffffe00512b7ab0
fast_syscall_common() at fast_syscall_common+0x101/frame 0xfffffe00512b7ab0
--- syscall (202, FreeBSD ELF64, sys___sysctl), rip = 0x800442b7a, rsp = 0x7fffffffb088, rbp = 0x7fffffffb0c0 ---
db> x/s version
version:        FreeBSD 13.0-CURRENT #40 r349025+3bdd0fc24f5b(mnowait-dbg)-dirty: Mon Jun 24 08:36:20 PDT 2019\012    root@vali.kishkinda.net:/usr/obj/usr/src/freebsd/amd64.amd64/sys/GENERIC\012
Comment 1 Mark Johnston freebsd_committer 2019-06-26 03:20:22 UTC
I fixed a similar problem in gmirror in r333278.  I suspect a similar approach would work for graid, but have not yet looked at the code.
Comment 2 Ryan Libby freebsd_committer 2019-06-26 05:27:30 UTC
(In reply to Mark Johnston from comment #1)

Thanks for the pointer, I will take a look.