rtsp: don't free the server handle while encoder threads still use it

[johnchia]

The bug

rtsp_finish() frees the server handle, but the encoder threads keep using it for a while afterwards.

main.c tears down in this order:

115:  rtsp_finish(rtspHandle);      <-- frees the handle
...
123:  region_stop();
126:  night_disable();
128:  sdk_stop();                   <-- encoder threads finally stop here

rtspHandle is never cleared, and media.c passes it straight into rtp_send_h26x() / rtp_send_mp3(). So every frame encoded between :115 and :128 dereferences freed memory.

The guard that should have caught this can't, because it lives in the memory it is checking:

if (gbl_get_quit(h->pool->sharedp->gbl)) {   /* h and h->pool are already freed */

rtsp_finish() raises that quit flag and then calls threadpool_delete(h->pool) and FREE(h). The check reads freed memory to decide whether the memory has been freed. region_stop() and night_disable() allocate in between, so the chunk gets reused, the check reads whatever now occupies that word, passes, and the sender goes on to rtsp_lock(h) on a destroyed mutex and list_map_inline(&h->con_list, ...) over a destroyed list.

DASSERT(h, ...) doesn't help — h is dangling, not NULL.

The fix

Move the shutdown flag into the handle itself, and stop freeing the handle in rtsp_finish():

• struct __rtsp_obj_t gains a finished flag, guarded by the existing mutex.
• rtsp_finish() raises it first, before releasing anything else.
• The two senders test h->finished instead of reaching through h->pool.
• rtsp_finish() no longer calls pthread_mutex_destroy() / FREE(h), so a late sender always finds a valid mutex and a flag telling it to turn around.

Connections, buffer pools, MIME buffers and the threadpool are all still released exactly as before. What stays allocated is the handle struct itself. rtsp_create() runs once per process, so that's a single small allocation reclaimed at exit. rtsp_create()'s own error path frees it explicitly, since nothing can reach it there yet.

Alternative worth considering

Calling sdk_stop() before rtsp_finish() in main.c would remove the hazardous window entirely and let the handle be freed normally. I didn't do that here because it changes shutdown ordering for every SoC and I can only test one — but if you'd prefer that shape, I'm happy to redo it.

Testing

Found on an Infinity6E (ssc30kq) running under Frigate with two streams: a SIGTERM restart segfaulted essentially every time, the faulting address landing a few dozen bytes into the sender's connection-list walk on a junk pointer. The equivalent fix has been running clean on that hardware since.

Cross-compiled clean for arm-openipc-linux-gnueabihf with no new warnings. Only the ssc30kq path is hardware-tested; the change is SoC-independent but a second pair of eyes on another target would be welcome.

Not addressed here

There's a narrower pre-existing race left in place: a sender that gets past the flag check can still be mid-send, holding transfer items, when rtsp_finish() deletes the pools. Closing it needs a drain barrier on the sender count. I left it out deliberately since I can't exercise it on this stack — happy to follow up separately if you want it.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

RTSP: avoid UAF by keeping server handle alive until encoder threads stop

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Prevent encoder threads from using a freed RTSP handle during process shutdown.
• Move the shutdown guard into the RTSP handle and check it in RTP send paths.
• Keep the handle/mutex alive after rtsp_finish(); still release pools, connections, and threadpool.

Diagram

graph TD
A["main.c shutdown"] --> B["rtsp_finish()"] --> C["Set h->finished"]
B --> D["Release con list"] --> E["Delete pools & threadpool"]
F["Encoder threads"] --> G["rtp_send_*()"] --> H["Check h->finished"] --> I["Bail out"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Reorder shutdown: call sdk_stop() before rtsp_finish()

• ➕ Eliminates the hazardous window so rtsp_finish() can safely free/destroy the handle as before
• ➕ Keeps handle lifetime semantics conventional (no intentional leak until exit)
• ➖ Changes shutdown ordering globally across SoCs; could introduce new platform-specific regressions
• ➖ May not be acceptable if other teardown steps depend on RTSP being closed early

2. Add a sender drain barrier / refcount on in-flight RTP sends

• ➕ Closes the remaining race where a sender passes the flag check but teardown deletes pools mid-send
• ➕ Allows eventual safe handle free/destroy without relying on process-exit reclamation
• ➖ More invasive: requires tracking in-flight operations and careful ordering to avoid deadlocks
• ➖ Higher review/maintenance burden for a shutdown-only edge case

Recommendation: The chosen approach (in-handle finished flag + keep handle/mutex alive) is a pragmatic, low-risk fix for the observed UAF without changing system-wide shutdown ordering. Consider a follow-up drain/refcount if pool-deletion races are seen in the field or if deterministic resource reclamation is required.

Files changed (3) +39 / -5

Bug fix (3) +39 / -5

rtp.cGuard RTP sends using handle-local finished flag +13/-2

Guard RTP sends using handle-local finished flag

• Replace the quit-flag check that reached through h->pool with a mutex-guarded check of h->finished. This prevents sender threads from reading freed threadpool memory and safely aborts late sends after shutdown begins.

src/rtsp/rtp.c

rtsp.cMark handle finished early and stop freeing handle in rtsp_finish() +22/-3

Mark handle finished early and stop freeing handle in rtsp_finish()

• rtsp_finish() now sets h->finished under the existing mutex before tearing down connections and pools, and it no longer destroys the mutex or frees the handle. The pool pointer is nulled after deletion; rtsp_create() error path explicitly destroys/frees the handle since it is not yet published.

src/rtsp/rtsp.c

rtsp.hAdd finished flag to RTSP handle +4/-0

Add finished flag to RTSP handle

• Extend struct __rtsp_obj_t with a mutex-guarded finished flag used by encoder-thread senders to detect shutdown without dereferencing threadpool internals.

src/rtsp/rtsp.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}