Bug #9112
openmulti-tenant: reload-tenants races on config tree and can deadlock
Description
The reload-tenants unix-socket command has two threading bugs.
1. Config tree race. Each detect loader thread loads its tenant's YAML with SCConfYamlLoadFileWithPrefix(), in parallel, into the global config tree, which has no locking. The reload prefix multi-detect.ID.reload.N sits under the tenant's original multi-detect.ID, and that node is removed when the original engine is freed. On the next reload, every loader re-creates its node under multi-detect at the same time, and an insert can be lost. The reload then fails with "failed to load yaml" or "failed to properly setup yaml", and the tree stays corrupted: later register-tenant commands fail too. TSAN confirms concurrent TAILQ_INSERT_TAIL calls from two loader threads (conf.c:99).
2. Deadlock. DetectLoaderSetupReloadTenants() holds master->lock while queuing tasks, which takes loader->m. A loader holds loader->m while running a task, which takes master->lock. When tenants share a loader (more tenants than multi-detect.loaders, default 4), reload-tenants hangs forever.
Also seen with TSAN: DetectEngineDeReference() decrements ref_cnt without the master lock, and this races with a rule reload on the main thread.
Workaround¶
Reload tenants one at a time with reload-tenant ID. Lowering multi-detect.loaders makes the deadlock more likely.
Fix¶
- Load all tenant YAML on the unix-socket thread before queuing loader tasks.
- Queue the reload tasks after releasing
master->lock.
Reproducer¶
Two Suricata-Verify tests, which need no TSAN build and don't hang when they fail:
multi-tenant-reload-race: 8 loaders; 30 cycles of registering 8 tenants and runningreload-tenantstwice. Fails 10/10 on main, and takes about 1s when fixed.multi-tenant-reload-deadlock: 14 tenants on 7 loaders,reload-tenants. Fails 20/20 on main.