Page MenuHomeFreeBSD

geom: fix initialization order
ClosedPublic

Authored by royger on May 3 2019, 2:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 6 2024, 7:36 PM
Unknown Object (File)
Dec 20 2023, 12:32 AM
Unknown Object (File)
Dec 17 2023, 10:26 AM
Unknown Object (File)
Oct 19 2023, 4:03 AM
Unknown Object (File)
Oct 19 2023, 1:13 AM
Unknown Object (File)
Aug 21 2023, 9:15 AM
Unknown Object (File)
Aug 7 2023, 8:12 PM
Unknown Object (File)
Aug 7 2023, 8:11 PM
Subscribers

Details

Summary

There's a race between the initialization of devsoftc.mtx (by devinit)
and the creation of the geom worker thread g_run_events, which calls
devctl_queue_data_f. Both of those are initialized at SI_SUB_DRIVERS
and SI_ORDER_FIRST, which means the geom worked thread can be created
before the mutex has been initialized, leading to the panic below:

wpanic: mtx_lock() of spin mutex (null) @ /usr/home/osstest/build.135317.build-amd64-freebsd/freebsd/sys/kern/subr_bus.c:620
cpuid = 3
time = 1
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe003b968710
vpanic() at vpanic+0x19d/frame 0xfffffe003b968760
panic() at panic+0x43/frame 0xfffffe003b9687c0
mtx_lock_flags() at mtx_lock_flags+0x145/frame 0xfffffe003b968810
devctl_queue_data_f() at devctl_queue_data_f+0x6a/frame 0xfffffe003b968840
g_dev_taste() at g_dev_taste+0x463/frame 0xfffffe003b968a00
g_load_class() at g_load_class+0x1bc/frame 0xfffffe003b968a30
g_run_events() at g_run_events+0x197/frame 0xfffffe003b968a70
fork_exit() at fork_exit+0x84/frame 0xfffffe003b968ab0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe003b968ab0

  • trap 0, rip = 0, rsp = 0, rbp = 0 ---

KDB: enter: panic
[ thread pid 13 tid 100029 ]
Stopped at kdb_enter+0x3b: movq $0,kdb_why

Fix this by initializing geom at SI_ORDER_SECOND instead of
SI_ORDER_FIRST.

Sponsored by: Citrix Systems R&D

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I suspect you'll need to bump some g_raid parts as well- it has modules declared at SI_SUB_DRIVERS/SI_ORDER_{FIRST,SECOND,THIRD} that I suspect could/should be bumped to SECOND/THIRD/FOURTH -- but I haven't had time to dig into it and examine the dependencies there.

I suspect you'll need to bump some g_raid parts as well- it has modules declared at SI_SUB_DRIVERS/SI_ORDER_{FIRST,SECOND,THIRD} that I suspect could/should be bumped to SECOND/THIRD/FOURTH -- but I haven't had time to dig into it and examine the dependencies there.

The only other usage of g_modevent I've found is in g_raid, but it's already initialized at SI_ORDER_THIRD:

https://svnweb.freebsd.org/base/head/sys/geom/raid/g_raid.c?view=markup#l2570

There may be more that I've missed, I'm not familiar with the geom code at all.

I can confirm this change fixes the panic.

I suspect you'll need to bump some g_raid parts as well- it has modules declared at SI_SUB_DRIVERS/SI_ORDER_{FIRST,SECOND,THIRD} that I suspect could/should be bumped to SECOND/THIRD/FOURTH -- but I haven't had time to dig into it and examine the dependencies there.

The only other usage of g_modevent I've found is in g_raid, but it's already initialized at SI_ORDER_THIRD:

https://svnweb.freebsd.org/base/head/sys/geom/raid/g_raid.c?view=markup#l2570

There may be more that I've missed, I'm not familiar with the geom code at all.

I can confirm this change fixes the panic.

Yeah, reading through, it looks to be safe. g_raid_md_modevent will trigger a retaste, but only if g_raid_started -- this is trivially false until at least g_raid_mod at SI_ORDER_THIRD, so it will only touch already initialized resources as long as it's before then. The same is true for g_raid_tr_modevent, but without the possible retaste. This looks to be safe.

This revision is now accepted and ready to land.May 3 2019, 2:54 PM
This revision was automatically updated to reflect the committed changes.