Project

General

Profile

Actions

Bug #9112

open
JI OD

multi-tenant: reload-tenants races on config tree and can deadlock

Bug #9112: multi-tenant: reload-tenants races on config tree and can deadlock

Added by Jason Ish 3 days ago. Updated 2 days ago.

Status:
Triaged
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

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 running reload-tenants twice. 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.

JI Updated by Jason Ish 2 days ago Actions #1

  • Status changed from New to Triaged
Actions

Also available in: PDF Atom