feat: recover FairPlay context after lease end - #4
Conversation
Stop treating endLeaseCb/pbErrCb as either fatal or ignorable. Enqueue async recovery, refresh the playback lease and FairPlay contexts under the existing kd_context_mutex, and refuse new decrypt/m3u8 work only while that reset is in flight.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7902f1535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (is_refreshing()) { | ||
| fprintf(stderr, "[.] decrypt sample aborted: lease recovery in progress\n"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
When a client passes this check and then blocks in readfull waiting for the sample body, the recovery worker can switch to Refreshing and call resetAllContexts before execution reaches NfcRKVnxuKZy04KWbdFu71Ou. The decrypt then uses the previously obtained kdContext after its backing FairPlay context has been reset, which can fail or access invalid state during ordinary lease recovery with active streams. An atomic state check cannot close this race; the reset must wait for in-flight decrypt operations (or otherwise retain their contexts) before invalidating them.
Useful? React with 👍 / 👎.
LYJW131
left a comment
LYJW131
left a comment
There was a problem hiding this comment.
方向是对的：44cbb9b 只是把「进程死掉」换成了「僵尸态活着」，这个 PR 补上了真正缺的那一半。把 FairPlay 调用挪出 lease-manager 回调线程、refresh_decrypt_ctx() 走 run_with_mutex 而不是自己 lock/unlock、rebuild 时直接调 getKdContextLocked 避免重入——这几个决定都和 6237205 立下的规矩一致，没有走回头路。不从上游 WorldObservationLog#62 抄 unlocked getKdContext 也是对的。
不过有一个我认为需要在合并前解决的问题，另外三个是次要的。
1. resetAllContexts() 会和在途解密竞争 → use-after-free（阻塞项）
kd_context_mutex 按设计只保护 context 构造，worker 拿到 kdContext 后整个 sample 循环是无锁跑的（main.c:441-443 的注释就是这么写的）。恢复线程持 kd_context_mutex 调 resetAllContexts() 时，解密线程并不持有这把锁，所以互斥不成立。sample 循环里那个 is_refreshing() 是 check-then-act——中间隔着一次可能阻塞很久的 readfull(connfd, sample, size)，原子变量只给可见性、不给互斥。
这是本 PR 新引入的：main 上 refresh_decrypt_ctx() 零调用者，resetAllContexts() 在运行时从未执行过。这个 PR 第一次让它跑起来，且正好跑在「有活跃解密流时租约到期」这个最容易撞上的场景。为了「不死」写的恢复路径反而变成新的崩溃源。
细节和三种修法写在 main.c:599 的行内评论里，我倾向 in-flight 计数 + 条件变量那种，最贴近现有结构。
2. refresh 期间到达的事件会立刻触发无退避的第二轮
endLeaseCb / pbErrCb 无条件入队（不像 request_lease_recovery() 会看 state）。成功路径把 consec_fails 清零，于是紧接着的那一轮完全跳过 sleep。requestLease 若自己会同步触发 endLeaseCb，就是零间隔自激循环。详见 main.cpp:160 行内。
3. m3u8 拒绝路径写 1 字节，既有失败路径写 2 字节
sizeof("\n") 是 2（含 \0）。同一函数里同一语义、两种线格式。main.c:802。
4. decrypt 拒绝是裸 EOF；Failed 期间照收请求
对称性和取舍问题，main.c:555 行内。
其它核对过、没问题的
refresh_decrypt_ctx签名从void ()改成int (void)，全树只有main.cpp:13一处声明，一致；没有遗留旧调用点。refresh_decrypt_ctx_locked里preshareCtx = NULL在resetAllContexts()之后、getKdContextLocked之前，所以 preshare 早退分支不会命中陈旧值；ready判据取preshareCtx != NULL也确实只在 key setup 真成功时为真。- 异常穿过
refresh_decrypt_ctx这个 C 帧的行为和现有handle()→handle_cpp那条路一致，锁由run_with_mutex释放。 main.cpp的 include 齐全（<unistd.h>对应sleep、<vector>/<queue>/<atomic>都有用到），enum class/%zu在 C++11 下没问题；CI 的 NDK r23b 交叉编译带-Wall -Werror是绿的。start_recovery_thread()在leaseMgr/FHinstance就绪之后调用；在那之前触发的回调只是入队，worker 起来后照常消费。- README 的描述和代码行为对得上（「refused only while that reset is in progress」确实只对应
Refreshing）。
第 1 条修掉我就没有别的意见了。要我直接在 agent/lease-recovery 上把这几处改了也可以，说一声。
Generated by Claude Code
| if (is_refreshing()) { | ||
| fprintf(stderr, "[.] decrypt sample aborted: lease recovery in progress\n"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
阻塞项：这个检查挡不住 resetAllContexts() 和在途解密的竞争，会 use-after-free。
kd_context_mutex 按设计只保护 context 的构造——main.c:441-443 的注释写得很明确：「Bound only context construction; workers still decrypt samples concurrently after acquiring their contexts.」
所以 worker 拿到 kdContext 之后就把锁放掉了，下面这段全程无锁：
if (is_refreshing()) return; // ← check
void *sample = malloc(size);
if (!readfull(connfd, sample, size)) // ← 阻塞在网络上，可以很久
...
NfcRKVnxuKZy04KWbdFu71Ou(*kdContext, 5, sample, sample, size); // ← use恢复线程那边：
g_recovery_state.store(RecoveryState::Refreshing);
refresh_decrypt_ctx();
→ run_with_mutex(&kd_context_mutex, refresh_decrypt_ctx_locked, ...)
→ resetAllContextsEv(FHinstance); // 销毁所有 context，包括 worker 手里那个run_with_mutex 拿的是 kd_context_mutex，而解密线程此刻没有持有它，所以这把锁在这里提供不了任何互斥。is_refreshing() 是典型的 check-then-act：中间隔着一次可能阻塞几百毫秒的 readfull，原子变量只保证可见性、不保证互斥。窗口一命中，*kdContext 就是对已释放内存的解引用——注意这里还是二次解引用，连 load 本身都可能直接 fault。
这个风险是本 PR 新引入的：main 上 refresh_decrypt_ctx() 零调用者（git grep 只有定义那一处），resetAllContexts() 在运行时从来没被执行过。这个 PR 第一次让它跑起来，而且恰好跑在「有活跃解密流时租约到期」这个场景下——也就是最容易撞上的时刻。结果是本来为了「不死」写的恢复路径，反而成了新的崩溃源。
建议的修法（要能真正排他，不能只靠标志位）：
- 加一个
pthread_rwlock_t fp_ctx_lock：worker 在整个 sample 循环持 rdlock，恢复线程持 wrlock 包住refresh_decrypt_ctx_locked。wrlock 同样要走run_with_mutex那套（换成 rwlock 版本的 guard），否则抛异常时照样泄锁。 - 光加 rwlock 还不够——worker 可能在 reset 之前就拿到了
kdContext，等 reset 做完再去拿 rdlock，用的还是野指针。所以要配一个 generation 计数：取 context 时记下 generation，每轮解密在 rdlock 内比对一次，变了就直接return让客户端重连。 - 更轻的做法是 in-flight 解密计数 + 条件变量：恢复线程在调
resetAllContexts()前先等计数归零（带超时），期间靠Refreshing挡住新请求。
第三种最贴近现有结构，改动也最小。
Generated by Claude Code
| if (ready) { | ||
| consec_fails = 0; | ||
| g_recovery_state.store(RecoveryState::Running); | ||
| fprintf(stderr, "[recovery] refresh ok, resuming\n"); | ||
| } else { | ||
| consec_fails++; | ||
| g_recovery_state.store(RecoveryState::Failed); | ||
| fprintf(stderr, "[recovery] refresh failed (consecutive=%d)\n", | ||
| consec_fails); | ||
| schedule_recovery(kRetryInternal); | ||
| } |
There was a problem hiding this comment.
refresh 期间到达的事件会立刻触发第二轮 refresh，而且不走退避。
endLeaseCb / pbErrCb 是无条件入队的（不像 request_lease_recovery() 会先看 state 是不是 Running）。所以在 Refreshing 那段窗口里 Apple 再吐一批 lease-end，全都进队列。
等这一轮成功返回：
if (ready) {
consec_fails = 0; // ← 退避计数清零
g_recovery_state.store(RecoveryState::Running);循环回到顶部，wait 因为队列非空立即返回，consec_fails == 0 于是 if (consec_fails > 0) 不成立、完全跳过 sleep，直接又是一轮 requestLease + resetAllContexts + key setup。
PR 描述里说的「coalesce bursts」只在单次 drain 窗口内成立，跨不过 refresh 本身那段时间。更糟的情况是 requestLease 自己会同步触发 endLeaseCb（账号真被吊销、反复 3084 时很可能）——那就是一个零间隔的自激循环：每轮都打一次 Apple、每轮都 resetAllContexts() 把 context 全掀掉、Refreshing 反复翻转导致 decrypt/m3u8 断续被拒。而且因为 FPS key 端点未必跟着挂，ready 完全可能一直是 1，consec_fails 永远清零，退避永远不生效。
两个方向，建议都做：
- refresh 成功后把队列里在本轮开始之前之后积压的事件丢掉——这一轮已经处理过了。用一个单调递增的 refresh generation：入队时带上当时的 generation，drain 时只认 generation ≥ 本轮起始值的事件。
- 不管成功失败都设一个最小 refresh 间隔（比如 1s）。现在的退避只在失败路径上，成功路径完全没有节流。
Generated by Claude Code
|
|
||
| if (is_refreshing()) { | ||
| fprintf(stderr, "[.] m3u8 request refused: lease recovery in progress\n"); | ||
| writefull(connfd, "\n", 1); |
There was a problem hiding this comment.
同一个函数里表达「这次没有 m3u8」的两条路径，写到线上的字节数不一样：
- 这里（拒绝）：
writefull(connfd, "\n", 1)→ 1 字节 - 下面 8 行（取失败）：
writefull(connfd, "\n", sizeof("\n"))→ 2 字节，sizeof("\n")把结尾的\0也算进去了
客户端如果是按行读，两者等价；但只要有任何一处依赖已有的 2 字节 framing，新路径就和既有失败路径对不上。既然本仓库现有的「失败」线格式就是 2 字节，这里跟着写 sizeof("\n") 最省事。
（1 字节其实才是更干净的那个，但那是另一件事——真要统一成 1 字节，得连下面那处一起改，并且按 AGENTS.md「不留兼容路径」的规矩把上报侧一起对齐，不要两条路各留一份。）
Generated by Claude Code
| if (is_refreshing()) { | ||
| fprintf(stderr, "[.] decrypt request refused: lease recovery in progress\n"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
小问题，两条拒绝路径的语义不对称：m3u8 那边至少回一个 \n，decrypt 这边直接 return，decrypt_worker_thread 随后 close(connfd)，客户端只看到协议中途一个裸 EOF。
站点侧因此没法区分「wrapper 正在恢复、等会儿重试」和「wrapper 挂了 / 解密失败」——两种情况在 socket 上长得一模一样。既然这个 PR 的卖点就是「HTTP servers stay up」，让调用方能看出来是暂时性拒绝才对得上。考虑回一个单字节的拒绝标记再关连接。
另外确认一下 Failed 状态的取舍：is_refreshing() 只在 Refreshing 为真，所以退避那 30 秒里请求照收，而此时 context 是已知失效的——客户端拿到的是静默失败，且 request_lease_recovery() 因为 state != Running 直接 no-op，连补一次恢复都不会。PR 描述里写了这是有意为之，那就算设计选择；只是长时间故障时，对外表现会是「一直在服务但一直解密不出来」，和 44cbb9b 之前那个僵尸态挺像的。要不要在 Failed 期间也拒收，值得再想一下。
Generated by Claude Code
resetAllContexts() frees every context the instance handed out. Decrypt
workers deliberately run unlocked -- kd_context_mutex bounds only context
construction, as the comment above it says -- so holding that mutex during
the reset excludes nothing: the workers are not holding it while they
decrypt. The is_refreshing() test in the sample loop did not close the gap
either. It is a check-then-act with a blocking readfull() of the sample
sitting inside the window, and an atomic gives visibility, not exclusion.
The result is a use-after-free on *kdContext, a double dereference, so even
the load can fault.
Nothing hit this before because refresh_decrypt_ctx() had no callers;
resetAllContexts() never ran. Wiring up recovery made it live, and made it
live precisely when streams are active and a lease expires -- the recovery
path added to stop the daemon dying became a way for it to crash.
Exclusion now has two halves, because the reset has to be safe against both
workers that are mid-decrypt and workers that were mid-decrypt before it
started:
- A barrier. begin_refresh_barrier() publishes Refreshing and waits for
g_active_decrypts to reach zero, both under g_ctx_mtx, so testing the
state and taking a slot cannot race. A slot brackets one decrypt call
with no socket I/O inside it, so the drain is quick; if it times out the
refresh is deferred and retried rather than run under a wedged call.
- A generation counter, bumped under kd_context_mutex next to the reset.
A worker captures it with its context under that same mutex, so the pair
cannot straddle a reset, and rechecks it per sample. That catches the
worker that acquired its context before the reset and resumes after it,
which the barrier alone cannot see.
Slot release lives in ~DecryptSlot for the same reason run_with_mutex
exists: the decrypt call can throw, main.c has no landing pads, and a
decrement written there would be skipped on the throw path, leaving the
recovery worker to wait out its full timeout on a slot never coming back.
Two smaller fixes:
Events queued during a refresh no longer trigger an immediate second one.
endLeaseCb/pbErrCb enqueue unconditionally, and success clears consec_fails,
so the backoff did not engage: a lease-end burst -- or a requestLease that
synchronously fires the callback -- spun refresh at full speed, tearing down
the context just rebuilt on every pass. A successful refresh now drops the
queue, since requestLease, resetAllContexts and a fresh preshare context all
happened after those events fired. A one-second floor between refreshes
backs that up independently of the failure backoff.
The m3u8 refusal path wrote one byte where the get-failure path eight lines
below writes sizeof("\n"), which is two. Both mean "no m3u8 this time" and
must not differ on the wire.
No wire-protocol or CLI changes. The refusal semantics of handle() and
handle_m3u8() are unchanged; a rejected decrypt still ends the connection,
which is how every other failure in handle() already signals the client.
|
1. UAF（阻塞项）排他现在分两半做，因为 reset 要同时对付「正在解密的 worker」和「reset 开始前就已在解密的 worker」：
槽位释放放在 sample 循环里那个 2. 自激循环refresh 成功后清空事件队列——队列里的事件都早于本轮 reset 完成， 3. m3u8 字节数改成 第 4 条没改，说明理由decrypt 协议里没有错误帧：客户端发完请求就等着读回恰好 一个行为变化recovery 完成后，仍持有旧 context 的 in-flight 解密会被拒绝并断开连接，客户端需要重新请求 key。这是有意的：拿刚 reset 过的 context 去重试同一个 sample，风险是静默解出错误数据，不如让它重连。日志里是 复核过的并发性质两把锁从不嵌套——worker 是 Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04ebae644f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "this refresh\n", | ||
| stale); | ||
| std::queue<int> drained; | ||
| g_recovery_q.swap(drained); |
There was a problem hiding this comment.
If endLeaseCb or pbErrCb enqueues a new failure after refresh_decrypt_ctx() finishes but before this queue is drained, that event is discarded even though it may describe the newly requested lease. The queue mutex only serializes access; it does not prove that every queued event predates the reset. The worker then transitions to Running without another retry, potentially leaving the daemon on a failed lease until an unrelated decrypt error schedules recovery, so only events captured before the refresh began should be treated as superseded.
Useful? React with 👍 / 👎.
2 participants
What changed
Lease-end and playback-error callbacks no longer
exit(1)and no longer ignore the event. They enqueue an async recovery; a dedicated worker coalesces bursts, re-requests the playback lease, resets FairPlay contexts, and rebuildspreshareCtxunder the existingkd_context_mutex.Incoming decrypt/m3u8 work is refused only while that reset is in progress (
Refreshing). Failed refreshes retry with bounded exponential backoff (1s → 2s → 5s → 10s → 30s). HTTP servers stay up.This is an adapted port of the idea in WorldObservationLog/wrapper#62, rewritten for this fork's concurrent decrypt workers and exception-safe mutex.
Why
44cbb9bstopped the process from dying on lease callbacks, butrefresh_decrypt_ctx()was never called. After Apple ends the lease (commonly code 3084), the wrapper stays listening with a stale FairPlay context: it looks healthy and decrypt/m3u8 keep failing.The worker exception path only called
requestLeaseand did not reset or rebuild contexts, so it could not recover either.How it works
endLeaseCb/pbErrCbonly enqueue and return. No library calls on the lease-manager thread.refresh_decrypt_ctx()from C++ withtry/catch.refresh_decrypt_ctx()runs viarun_with_mutexsorequestLease/resetAllContexts/ key setup cannot leakkd_context_mutexif they throw.getKdContextLockeddirectly to avoid re-entering the mutex.handle()/handle_m3u8()fail fast only duringRefreshing. Scheduled/Failed still accept requests.Running, so an in-flight failure during reset does not start a second cycle.Intentionally not taken from upstream WorldObservationLog#62
drm-statefile (no consumer in this repo)wrapper-rootless.cnamespace rewriteDockerfile.build/ drift-check CIgetKdContext(would regress Serialize FairPlay context creation #1 /6237205)Impact
Validation
clang++ -std=c++11 -Wall -Werror -fsyntax-only main.cppon the authoring hostbuildjob is the compile check