Skip to content

fastrpc: fix use-after-free in close_domain_session during session teardown - #365

Open
Jianping (Jianping-Li) wants to merge 1 commit into
qualcomm:developmentfrom
Jianping-Li:close_session
Open

fastrpc: fix use-after-free in close_domain_session during session teardown#365
Jianping (Jianping-Li) wants to merge 1 commit into
qualcomm:developmentfrom
Jianping-Li:close_session

Conversation

@Jianping-Li

@Jianping-Li Jianping (Jianping-Li) commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

close_domain_session() iterated over the per-domain handle lists (nql, rql and ql) using QLIST_NEXTSAFE_FOR_ALL, which caches the next node (pnn) up front. Inside the loop body the list mutex is dropped to issue the remote close:

pthread_mutex_unlock(&hlist[domain].lmut);
remote_handle64_close(hi->local);
pthread_mutex_lock(&hlist[domain].lmut);

NEXTSAFE only guarantees that freeing the current node (pn) is safe; it does not protect the cached next node (pnn) across the unlock window. While the lock is dropped, the cascaded cleanup inside the remote close path and the concurrently exiting listener thread can unlink and free the node pnn points to. On the next iteration pn = pnn dereferences a dangling pointer, leading to a use-after-free crash:

close_domain_session
remote_session_control (req=7, FASTRPC_SESSION_CLOSE)
Program terminated with signal SIGSEGV
pnn = 0xaaa1b7150f3d   /* poisoned next pointer */

This only reproduces on targets that support status notifications: the FASTRPC_SESSION_CLOSE path in the test app is gated by status_notif_capability, so targets without notification support never enter close_domain_session and are unaffected.

Replace the NEXTSAFE traversal with a pop-based loop that re-reads the list head under the lock on every iteration and unlinks the node with QList_Pop before dropping the mutex. The node is off the list before the remote close runs, so neither the close cascade nor the listener thread can free a node we still reference, eliminating the dangling next pointer. Apply the same fix to all three lists (nql, rql, ql).

@github-actions

Copy link
Copy Markdown

qualcomm/fastrpc.triage This pull request has been marked as stale due to 30 days of inactivity.

@github-actions github-actions Bot added the Stale label Aug 16, 2026
Comment thread src/fastrpc_apps_user.c Outdated
…ardown

close_domain_session() iterated over the per-domain handle lists (nql,
rql and ql) using QLIST_NEXTSAFE_FOR_ALL, which caches the next node
(pnn) up front. Inside the loop body the list mutex is dropped to issue
the remote close:

    pthread_mutex_unlock(&hlist[domain].lmut);
    remote_handle64_close(hi->local);
    pthread_mutex_lock(&hlist[domain].lmut);

NEXTSAFE only guarantees that freeing the current node (pn) is safe; it
does not protect the cached next node (pnn) across the unlock window.
While the lock is dropped, the cascaded cleanup inside the remote close
path and the concurrently exiting listener thread can unlink and free
the node pnn points to. On the next iteration pn = pnn dereferences a
dangling pointer, leading to a use-after-free crash:

    close_domain_session (fastrpc_apps_user.c:2284)
    remote_session_control (req=7, FASTRPC_SESSION_CLOSE)
    Program terminated with signal SIGSEGV
    pnn = 0xaaa1b7150f3d   /* poisoned next pointer */

This only reproduces on targets that support status notifications: the
FASTRPC_SESSION_CLOSE path in the test app is gated by
status_notif_capability, so targets without notification support never
enter close_domain_session and are unaffected.

Replace the NEXTSAFE traversal with a pop-based loop that re-reads the
list head under the lock on every iteration and unlinks the node with
QList_Pop before dropping the mutex. The node is off the list before the
remote close runs, so neither the close cascade nor the listener thread
can free a node we still reference, eliminating the dangling next
pointer. Apply the same fix to all three lists (nql, rql, ql).

Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants