[audit][quality] Documented, unfixed use-after-free race between TSTextLogObjectCreate and log config reload

Severity: high · Category: race · ✓ adversarially verified
Location: src/proxy/logging/Log.cc:133

What's wrong

Log::change_configuration() swaps the active LogConfig and then releases the old manager's API mutex. A comment in the code itself states that a plugin calling TSTextLogObjectCreate() concurrently can register its LogObject with the old (soon-to-be-freed) LogConfig, and that Traffic Server 'would crash the next time the plugin referenced the freed object'. This is a known crash race on the config-reload path that has been shipped as an XXX comment instead of being fixed or asserted against; config reload plus API log objects is a realistic production combination.

Evidence

// XXX There is a race condition with API objects. If TSTextLogObjectCreate()
// is called before the Log::config swap, then it will be blocked on the lock
// on the *old* LogConfig and register it's LogObject with that manager. If
// this happens, then the new TextLogObject will be immediately lost. Traffic
// Server would crash the next time the plugin referenced the freed object.

Suggested fix

Close the window: after the ink_atomic_swap, re-check the old manager for objects registered during the race and migrate them to the new LogConfig before the old one is scheduled for deletion (the API mutex is already held, so a migration loop there is safe), or route TSTextLogObjectCreate through configProcessor's refcounted get() so it always registers against the current config.

Verification

This finding was independently re-traced by a second reviewer instructed to refute it; it was confirmed. Reasoning:

Traced end-to-end. TSTextLogObjectCreate (InkAPI.cc:6506-6534) reads the raw global Log::config with no refcount and calls manage_api_object, which blocks on the old manager's _APImutex (LogObject.cc:943). Log::change_configuration (Log.cc:111-152) runs on the log preproc thread (not continuation-dispatched, so EThread mutex protection does not apply), holds that same _APImutex while transfer_objects copies only already-registered API objects, swaps Log::config, and releases the mutex — the blocked creator then registers its object into the OLD manager's _APIobjects with refcount 1, after the transfer. configProcessor.set schedules deletion of the old LogConfig, whose ~LogObjectManager refcount_dec's the orphaned API object to 0 and deletes it, while the plugin still holds the raw pointer. Guards searched for and absent: no re-read of Log::config after mutex acquisition, no generation counter, no plugin-held refcount, no shared continuation mutex. The XXX comment accurately describes live behavior. Caveats: narrow window; crash is deferred until the ConfigProcessor release timeout fires.

Filed from an automated multi-lens codebase audit. Full report: CODEBASE_AUDIT.md / audit-report.html on branch claude/codebase-audit-review-9nw7vz.

Read src/proxy/logging/Log.cc:111-152 alongside InkAPI.cc:6506-6534 and LogObject.cc:943, then trace the config-reload and TSTextLogObjectCreate interleaving described in the report. Verify the chosen fix closes the old-manager registration window and that an API log object remains valid after reload; the payload names no regression test.